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

Switch CI from Azure to GitHub actions #82

Merged
merged 11 commits into from
Apr 6, 2022
Merged

Conversation

dhomeier
Copy link
Contributor

Description

Transition using the reusable workflows from https://github.com/OpenAstronomy/github-actions-workflows

@codecov
Copy link

codecov bot commented Mar 25, 2022

Codecov Report

Merging #82 (63d20d9) into main (1e86d98) will decrease coverage by 2.20%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #82      +/-   ##
==========================================
- Coverage   74.18%   71.97%   -2.21%     
==========================================
  Files          18       18              
  Lines         860      860              
==========================================
- Hits          638      619      -19     
- Misses        222      241      +19     
Impacted Files Coverage Δ
glue_wwt/viewer/tests/test_wwt_widget.py 97.47% <100.00%> (-2.53%) ⬇️
glue_wwt/viewer/tools.py 50.94% <0.00%> (-30.19%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e86d98...63d20d9. Read the comment docs.

@dhomeier dhomeier force-pushed the gh-actions branch 2 times, most recently from 1a027a9 to edb2c88 Compare March 25, 2022 13:09
@dhomeier dhomeier force-pushed the gh-actions branch 2 times, most recently from 2302691 to 2b5a9cd Compare March 25, 2022 13:34
@dhomeier dhomeier force-pushed the gh-actions branch 4 times, most recently from c381ace to ce17c42 Compare March 25, 2022 13:56
@dhomeier dhomeier marked this pull request as draft March 25, 2022 14:40
@dhomeier dhomeier closed this Mar 25, 2022
@dhomeier dhomeier reopened this Mar 25, 2022
@dhomeier dhomeier marked this pull request as ready for review March 25, 2022 21:37
@dhomeier dhomeier closed this Mar 25, 2022
@dhomeier dhomeier reopened this Mar 25, 2022
@astrofrog
Copy link
Member

@dhomeier - I think we should skip the tour saving test until we figure out #84 - that way we can move forward with the migration anyway.

@dhomeier
Copy link
Contributor Author

dhomeier commented Apr 5, 2022

Yes, just thought so as well – and the Windows jobs seemed to be stuck for 15 minutes after the actual tox runs again.

@dhomeier dhomeier force-pushed the gh-actions branch 3 times, most recently from f4930cf to dcb802e Compare April 5, 2022 15:20
@dhomeier
Copy link
Contributor Author

dhomeier commented Apr 5, 2022

Mysteriously the Windows runs are no longer hanging – or could it have been the switch to v1.0.2?
I'd still feel safer if there was a shorter timeout...

@dhomeier
Copy link
Contributor Author

dhomeier commented Apr 5, 2022

@astrofrog I don't know if there is a syntax to pull the dev branch of the workflows for testing if the fail-fast setting works as I believe it would, but that can certainly wait for a v1.0.3 release as well.

The Shapely 1.8.0 installations on macOS and Windows py310 seem to have similar geos_c problems – in fact the other Pythons are already on 1.8.1post1, so maybe that is the bugfix.

@astrofrog
Copy link
Member

You can always replace @v1 by @main when calling the workflows to get the very latest version

@dhomeier
Copy link
Contributor Author

dhomeier commented Apr 5, 2022

OK, that would be better than accidentally staying stuck at @v1.0.2 in any case.
Hmm, so @v1 currently is @v1.0.2; so that was obviously not what unstuck the windows runs.

@dhomeier
Copy link
Contributor Author

dhomeier commented Apr 6, 2022

So mac and win py310 have updated there Shapely as well.

@astrofrog
Copy link
Member

Ok thanks! You can now revert back to using the default fast-fail as v1.0.3 of the workflows is out.

Also I think we can drop Python 3.6 as glue-core requires 3.7 (https://github.com/glue-viz/glue/blob/main/setup.cfg#L26) which might help resolve the remaining CI issues. Can you update this in setup.cfg too?

@dhomeier
Copy link
Contributor Author

dhomeier commented Apr 6, 2022

Ok thanks! You can now revert back to using the default fast-fail as v1.0.3 of the workflows is out.

Already used in the last run (set in tests to true and in allowed_failures to false, though the py310 arguably could be moved back to the "required" tests now ;-).
Well, with py36 gone that would not leave any allowed failures at the moment.

@astrofrog
Copy link
Member

I think we should just remove the fast-fail options from our config now - the default of 'false' is what we want I think

@dhomeier
Copy link
Contributor Author

dhomeier commented Apr 6, 2022

The default was true – at least for all envs in the same job; therefore I wanted this for the allowed failures.
false arguably would make debugging more tedious, but otoh avoid burning CI time if it's always the same env failing.

I think the middle path recommended for astropy coordinated packages was to define a smaller set of mandatory tests that first have to pass (actually not starting the full set before that, which seems not possible here, but at least anything failing early on on setup or so would cancel the remaining jobs soon). But for this repo perhaps not so useful, since there are no environments with minimal deps or dev/very extended tests.

Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

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

Looks good! I agree about trying to follow something more like what we do in astropy with different stages of testing, though packages like this are low traffic enough that I'm not sure if it's worth the extra complexity. Fast failing is not a good option in any case as if you run the CI again you might not have the same job fail which makes debugging impossible.

@astrofrog astrofrog merged commit 101e94a into glue-viz:main Apr 6, 2022
@dhomeier dhomeier deleted the gh-actions branch April 7, 2022 00:08
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.

2 participants