Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix autotest failures on main #1627

Merged
merged 12 commits into from
Sep 12, 2024

Conversation

phargogh
Copy link
Member

@phargogh phargogh commented Sep 11, 2024

This PR moves the multiprocessing freeze support call to __main__.py, which addresses the autotest issue as demonstrated by the GHA logs here: https://github.com/phargogh/invest/actions/runs/10821260328/job/30022930272 (I had to force the autotest to happen since it doesn't usually run on most pushes).

I also noticed a bunch of warnings related to outdated versions of actions steps, so I just went ahead and updated those as well.

Fixes #1622

Checklist

- [ ] Updated HISTORY.rst and link to any relevant issue (if these changes are user-facing)
- [ ] Updated the user's guide (if needed)
- [ ] Tested the Workbench UI (if relevant)

@phargogh phargogh self-assigned this Sep 11, 2024
@phargogh phargogh marked this pull request as ready for review September 12, 2024 05:29
Copy link
Contributor

@davemfish davemfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @phargogh . I had one question in __main__.py but doesn't require any changes.

And there's a breaking change in one of the actions that we need to address. Thanks for updating all of those for us!

import sys

# We want to guarantee that this is called BEFORE any other processes start,
# which could happen at import time.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If another process was starting at import time, would that be an indication that the code starting that process belongs under an if __name__ == '__main__' ? Maybe it's outside of our control though?

@@ -196,7 +196,7 @@ jobs:
- name: Check long description with twine
run: twine check $(find dist -name "natcap[._-]invest*")

- uses: actions/upload-artifact@v3
- uses: actions/upload-artifact@v4
with:
name: Source distribution
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there's a breaking change here: https://github.com/actions/upload-artifact?tab=readme-ov-file#breaking-changes

I guess we'll have to rename the artifact. Using matrix.os and matrix.python-version should do it.

@davemfish davemfish merged commit af8ce8b into natcap:main Sep 12, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

invest-autotest fails on main
2 participants