-
Notifications
You must be signed in to change notification settings - Fork 54
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 .deb package publishing to S3 #314
Conversation
def lock(self, conf, baseuri): | ||
"""Rename semaphore file""" | ||
print(f"Locking {baseuri} ...") | ||
cmd = f"s3cmd mv --no-progress {baseuri}/.debrepos3.nolock {baseuri}/.debrepos3.lock" |
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.
Be careful that the mv operation is not atomic on s3 and is basically equivalent to doing a copy and delete. It might still be enough to prevent race conditions on the CI which has high latency, but it's probably worth noting this caveat in the code to prevent surprises down the line.
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.
The copy + delete process should implement something similar to a 2-step lock: during the interval when both files exist other attempt at copy should fail.
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.
Left a minor comment, but otherwise looks good to me.
Thank you, @FedericoCeratto! I will look into merging this package after 3.10.0-beta is out. (In 3.10.0-beta we fix a bunch of Android-related issues and, once that is done, we can prepare 3.10.0's |
This diff implements part of the release checklist at ooni/probe#1439. The plan is to bless a beta release and use it for further testing on Android devices. Afterward, we need to apply some extra changes to the `cli` (including #314 and #312). Finally, we will bless a full 3.10.0 release.
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.
Thanks, I am going to merge this work soon!
Not merged yet because it seems we still have some issues with the bucket. |
This diff implements part of the release checklist at ooni/probe#1439. The plan is to bless a beta release and use it for further testing on Android devices. Afterward, we need to apply some extra changes to the `cli` (including ooni#314 and ooni#312). Finally, we will bless a full 3.10.0 release.
This PR switches the` .deb` package publishing to s3 because Bintray has been shut down.
WIP