-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update skare3-promote to handle osx-arm64 packages #115
Conversation
Today I used this to promote ska3-perl. |
Terrific. I also ended up promoting some packages manually last week because I wasn't sure this was done. It seems like should add a rule that skare3_tools PRs need approval if we want to use it as part of the non-fsds process so I'll do that now. |
I have to say... the promotion did fail to promote the right build of perl-ska-classic 0.6, but that is a different issue than what is done in this PR. |
skare3_tools/packages.py
Outdated
@@ -167,21 +167,18 @@ def wrapper(*args, update=False, **kwargs): | |||
if update_policy is not None and result is not None: | |||
update = update or update_policy(filename, result) | |||
if not dir_access_ok(filename): | |||
if result is None: | |||
raise Exception( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this removed because the exception has changed or?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I had not noticed this was in this branch. This is a change unrelated to the promotion script. This is used in the packages
module and anything that uses it (like the skare3-changes-summary
script).
Now it raises no exception. Before this change, if the user had no write access to the cache file, it would raise an exception, but this is not needed. I changed the logic so it would always tries to read. At this point it only checks access, and uses that information below (right before writing).
It can still raise an exception if the cache file exists and it has no read access.
I might just edit the PR description to make it a larger PR instead of rebasing. I'll check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. Fine with an unrelated fix if in the description - though might want to add a note about testing this.
I looked into these "extra" changes again, and it turns out they are older changes that are already in master. I just made the mistake of starting on an old branch. I rebased on master locally, for which I had to get rid of the last commit (ruff). I then applied one of the changes in this last ruff commit. I can force-push that, but then you are going to ask me to test again... |
Since the testing was done "live" with this branch and the packaging fixes are unrelated, I don't think there would be anything to re-test, so I'm fine with a rebase. As-is this PR has merge conflicts so I think that is needed to move it forward anyway. |
c0e9863
to
da6796d
Compare
I just force-pushed the rebased branch |
Description
Adds osx-arm64 to platforms to promote.
I just used this to promote packages to the test channel using PYTHONPATH.
Interface impacts
Testing
Unit tests
Functional tests
This was live-tested to promote ska3-perl 2024.7.