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

Publish distributions to TestPyPI on each day #2220

Merged
merged 12 commits into from
Jan 22, 2021

Conversation

HideakiImamura
Copy link
Member

@HideakiImamura HideakiImamura commented Jan 18, 2021

Depends on #2238.

Motivation

Currently, we publish distributions to PyPI and TestPyPI at the same time when the release, so we cannot detect any error on publishing to PyPI beforehand. This PR proposes to publish to TestPyPI on each master push. It is a room of discussion for the trigger of the publish.

I have changed the uploads to be done every weekday. See #2220 (comment).

@HideakiImamura HideakiImamura added the CI Continuous integration. label Jan 18, 2021
@HideakiImamura HideakiImamura added this to the v2.5.0 milestone Jan 18, 2021
@codecov-io
Copy link

codecov-io commented Jan 18, 2021

Codecov Report

Merging #2220 (dd1e71a) into master (8b1dc9a) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2220   +/-   ##
=======================================
  Coverage   92.19%   92.19%           
=======================================
  Files         124      124           
  Lines       10240    10241    +1     
=======================================
+ Hits         9441     9442    +1     
  Misses        799      799           
Impacted Files Coverage Δ
optuna/trial/_frozen.py 96.03% <0.00%> (ø)
optuna/testing/storage.py 96.96% <0.00%> (+0.09%) ⬆️

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 8b1dc9a...dd1e71a. Read the comment docs.

@toshihikoyanase
Copy link
Member

@not522 Could you review this PR if you have time?

@hvy Please feel free to add comments if you have any.

Copy link
Member

@toshihikoyanase toshihikoyanase left a comment

Choose a reason for hiding this comment

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

Thank you for your PR. More frequent upload to TestPyPI will prevent configuration error like this one.

I wonder if we can upload the packages with the same names multiple times. I think the duplicated package was refuled by PyPI.

.github/workflows/pypi-publish.yml Outdated Show resolved Hide resolved
@HideakiImamura
Copy link
Member Author

I wonder if we can upload the packages with the same names multiple times. I think the duplicated package was refused by PyPI.

You are right. From here, we cannot re-upload the same version to PyPI and it is the same for TestPyPI from here. I think we have to create a different branch changing the version dependent on the commit hash (if the trigger is the master push).

@HideakiImamura
Copy link
Member Author

@toshihikoyanase The upload on each master push seems to be impossible since the version name should be different for each upload and the version name cannot include the commit hash according to PEP440.

I have changed the trigger to upload TestPyPI each weekday. PTAL.

@HideakiImamura HideakiImamura changed the title Publish distributions to TestPyPI on the master push Publish distributions to TestPyPI on each weekday Jan 19, 2021
@HideakiImamura
Copy link
Member Author

HideakiImamura commented Jan 19, 2021

I confirmed the TestPyPI upload succeeded not by the scheduled event but by the push event. See here

Copy link
Member

@not522 not522 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@toshihikoyanase toshihikoyanase left a comment

Choose a reason for hiding this comment

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

LGTM! This change enables users to try the latest Optuna easily in addition to preventing configuration errors of the CI. Thank you.

@toshihikoyanase
Copy link
Member

I'll merge this PR after we resolve #2220 (review).

@HideakiImamura
Copy link
Member Author

@toshihikoyanase @not522 Thanks for your careful reviews! PTAL.

@HideakiImamura HideakiImamura changed the title Publish distributions to TestPyPI on each weekday Publish distributions to TestPyPI on each day Jan 22, 2021
Copy link
Member

@toshihikoyanase toshihikoyanase left a comment

Choose a reason for hiding this comment

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

Thank you for your update. LGTM again.

@toshihikoyanase
Copy link
Member

@not522 Can I merge the PR? Or, do you check the change 97dbf88?

@not522
Copy link
Member

not522 commented Jan 22, 2021

LGTM, please merge it.

@toshihikoyanase
Copy link
Member

Thanks!

@toshihikoyanase toshihikoyanase merged commit 23eba5a into optuna:master Jan 22, 2021
@HideakiImamura HideakiImamura deleted the ci/test-pypi-publish branch June 9, 2023 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants