Repository navigation
CI: add reference lowest env solution + script to check its evolutions - #19022
Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
26a3f34 to
489b61e
Compare
|
Hum, CI is hitting another edge cache around the I don't know why I'm not seeing this locally. Will try to solve this after lunch. |
25a9a3c to
8e22f4e
Compare
|
I'm not convinced the error message even makes sense. I don't see why having a dependency that itself depends on the project should make requirement unsatisfiable (it this was true, dependency cycles wouldn't be allowed at all ?). I will take this upstream for now. |
|
reported at astral-sh/uv#16978 update: my mistake. Resolved locally. |
1b7c5c6 to
a7a4553
Compare
|
Interesting, now I'm getting what looks like a genuine failure that I seemingly cannot reproduce locally. |
|
A, maybe I got it. |
88ac288 to
5946375
Compare
|
Turned out to be a red herring. Instead, the problem was I had a residual, botched |
5946375 to
5bf3fa7
Compare
9443965 to
8786859
Compare
|
rebased to resolve a merge conflict @Cadair, do you think you could have another look at this reasonably soon ? |
8786859 to
85bfe95
Compare
46b32b5 to
cbc9984
Compare
cbc9984 to
322b3c0
Compare
|
I finally got RTD green. |
pllim
left a comment
There was a problem hiding this comment.
I probably blocked merge initially to await more maintainers to chime in but since there are now 2 approvals, I guess we can merge and see what happens. But before that, I have some comments.
Also if you all want to think about ways this could compromise GHA security, now is the time to do it. Recent attacks: https://www.stepsecurity.io/blog/hackerbot-claw-github-actions-exploitation
7f7bb04 to
e8ea105
Compare
| @@ -0,0 +1,467 @@ | |||
| astropy | |||
| ├── astropy-iers-data v0.2026.2.23.0.48.33 | |||
There was a problem hiding this comment.
Is this going to have to be updated every week?
There was a problem hiding this comment.
yes. But I'm planning to automate it in #19380 :)
There was a problem hiding this comment.
(it'll be as simple as running the script before changes are committed)
There was a problem hiding this comment.
@astrofrog do you have any concern here with the answers above?
Co-authored-by: P. L. Lim <[email protected]>
e8ea105 to
91f9513
Compare
Co-authored-by: P. L. Lim <[email protected]>
I'm not a security expert so evidently this isn't worth much but I cannot forsee how this would create an attack vector. I do try to stay as paranoid as my mental capacity allows though 😄 |
|
Thanks for addressing my comments. I think we just waiting on #19022 (review) now from Tom R. |
|
Don't have time to review in detail myself but if there are two approvals that's fine by me |
Description
based off #18955
close #18965
There are several options for "locking" the resolved environment:
pylock.oldest.toml)pip freezeorpip listuv treeboth 1) and 2) have portability issues because the exact result would be platform dependent, so I went with 3) instead, which can be made cross-platform with a
--universalresolution strategy.Option 3) also comes with what I think is a great benefit: it makes the relationship between all our dependencies apparent, immediately answering mundane but otherwise time consuming questions like "where does this one even come from ?". Also, storing this graph in a legible form helps demystifying dependency hell, which is a nice side effect.
I still need to add the CI job to actually run the script, but the script itself should already be stable.I intentionally introduced a mistake in the reference file to make sure CI fails as expected when I do plug it in.
I plan to finish this tomorrow.
note to self: I want to run the script through
pipx, notuv, just to be absolutely sure that it doesn't use a system level install of uv.