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

Convert facebookresearch/hydra to GitHub Actions #2973

Merged

Conversation

jesszzzz
Copy link
Contributor

@jesszzzz jesszzzz commented Oct 22, 2024

Pipeline migrated from CircleCI 🎉

PR originally generated with gh actions-importer migrate circle-ci --target-url https://github.com/facebookresearch/hydra.git --output-dir migrate --circle-ci-project hydra

Added the following changes:

  • Removed the no longer used AWS and CIRCLE_CI key environment variables
  • Removed plugin only test logic, now we just always run all the tests - looks like there was some kind of logic to only run the pipeline tests in certain cases to handle permissions (discussion in avoid running plugin tests on forked PRs #2180) which no longer seem relevant since we're removing these tokens entirely
  • Fixed up setup steps to work correctly for linux, windows, and mac
  • Fixed the tests to work on windows properly
    • Added quotes around temp directory override since the windows directory contains ~ which messes up the parser without the quotes
    • Forced tests to use the C: drive
    • Forced tests to output in utf-8 (the github windows image seems to be ignoring the PYTHONIOENCODING env var

Note there's probably still some inefficiency in this setup but not a priority to improve it right now.

Test:
See CI signals. Note the CircleCI failure is expected and we'll disable it once this PR is approved.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 22, 2024
@jesszzzz jesszzzz force-pushed the convert-facebookresearch-hydra-to-actions-20241022-163241 branch 19 times, most recently from 5446b87 to 03a85cc Compare October 24, 2024 17:26
@jesszzzz jesszzzz force-pushed the convert-facebookresearch-hydra-to-actions-20241022-163241 branch from 03a85cc to b21b933 Compare October 24, 2024 17:26
@jesszzzz jesszzzz force-pushed the convert-facebookresearch-hydra-to-actions-20241022-163241 branch 2 times, most recently from 6f3d326 to b48d803 Compare October 25, 2024 14:45
@jesszzzz jesszzzz force-pushed the convert-facebookresearch-hydra-to-actions-20241022-163241 branch 3 times, most recently from c8d8411 to 2455551 Compare October 25, 2024 18:10
@jesszzzz jesszzzz force-pushed the convert-facebookresearch-hydra-to-actions-20241022-163241 branch 3 times, most recently from 4d87351 to 0dff57c Compare October 25, 2024 20:44
@jesszzzz jesszzzz force-pushed the convert-facebookresearch-hydra-to-actions-20241022-163241 branch from 0dff57c to 3e9ba4c Compare October 25, 2024 21:20
Copy link
Contributor

@Sanjay-Ganeshan Sanjay-Ganeshan left a comment

Choose a reason for hiding this comment

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

Mostly changing double quotes to single quotes

@jesszzzz jesszzzz merged commit 103a99a into main Oct 28, 2024
24 of 25 checks passed
@jesszzzz jesszzzz deleted the convert-facebookresearch-hydra-to-actions-20241022-163241 branch October 28, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants