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

BUG: fix parsing upload time #9

Merged

Conversation

neutrinoceros
Copy link
Contributor

This adresses an issue noticed downstream at astropy/astropy#16076, where a crash would occur when cleaning old wheels as the upload time stamp wouldn't match the expected format.
I have limited understanding of everything that's going on here so this might not be the correct approach, but it seemed simple enough. I've assumed that both the old and new timestamp format could still be in use so I wouldn't break backward compatibility, but maybe that's not important ?

@Cadair
Copy link
Member

Cadair commented Feb 20, 2024

Thanks @neutrinoceros I will let @astrofrog or @ConorMacBride look over this as they understand it unlike me!

@Cadair Cadair requested a review from astrofrog February 20, 2024 09:08
@pllim
Copy link

pllim commented Feb 20, 2024

I don't understand the failure but doesn't seem related?

Error:  ('Authorization header was not given', 401)
Error: Process completed with exit code 1.

Thanks for the quick fix! Hopefully it can be merged soon. If I have to wait a while to be able to upload a nightly wheel again, I might take Brigitta's suggestion to try out the Scientific Python workflow (xref #7). So please let me know if you are unable to get to this by the end of the week. Thanks!

@pllim
Copy link

pllim commented Feb 20, 2024

p.s. I wonder if there is a better way to future-proof in case the server changes the time formatting again.

@ConorMacBride
Copy link
Member

Thanks @neutrinoceros! As @pllim suggests it would be good to future-proof the parsing, but we can decide that in a different PR so we get the fix out now.

One option would be to use the dateutil package to parse without specifying a format. Another would be to use regex to extract just a date ([0-9]{4}-[0-9]{2}-[0-9]{2}) as the time is unlikely to be necessary in most cases.

@ConorMacBride
Copy link
Member

It is only one specific wheel build that has the different time format (astropy-6.1.dev455+gaeeed0886e-cp39-cp39-musllinux_1_1_x86_64.whl from 2024-02-16 04:19:43+00:00). Everything else is still using the existing format including the latest uploads.

The test failures in this PR are due to PRs not having access to secrets.

@neutrinoceros
Copy link
Contributor Author

I tend to prefer solutions that don't require new dependencies so if a regexp suffices, I can craft a more general solution tomorrow !

@ConorMacBride ConorMacBride merged commit 612ea80 into OpenAstronomy:main Feb 20, 2024
0 of 2 checks passed
@neutrinoceros neutrinoceros deleted the fix_upload_time_parsing branch February 20, 2024 20:03
@pllim
Copy link

pllim commented Feb 20, 2024

Moment of truth is approaching... https://github.com/astropy/astropy/actions/runs/7979290389

@ConorMacBride ConorMacBride linked an issue Feb 20, 2024 that may be closed by this pull request
@pllim
Copy link

pllim commented Feb 20, 2024

dev483 uploaded successfully. Thanks, all!

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.

Wheel upload failed with time format error
4 participants