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

Set up GitHub release token for Travis CI #7485

Closed
hugovk opened this issue Oct 23, 2023 · 30 comments · Fixed by #7690
Closed

Set up GitHub release token for Travis CI #7485

hugovk opened this issue Oct 23, 2023 · 30 comments · Fixed by #7690
Assignees
Labels
Milestone

Comments

@hugovk
Copy link
Member

hugovk commented Oct 23, 2023

For #7390.
Re: #7348 (comment), python-pillow/pillow-wheels#156 (comment).

We need to set up a token for Travis CI to be able to upload wheels to this repo's release section.

  1. Create a GitHub personal access token scoped to this repo. ⚠️ A repo admin needs to do this due to permissions.
  2. Install Travis CLI:
sudo gem install travis --no-document
...travis --version
Shell completion not installed. Would you like to install it now? |y| y
1.11.1
  1. The next bit is where I got stuck, but try something like this (https://stackoverflow.com/a/76234874/724176):
❯ travis login --github-token GITHUB_PERSONAL_ACCESS_TOKEN

❯ travis setup releases -r python-pillow/Pillow 
  1. Save the new token as GITHUB_RELEASE_TOKEN at https://app.travis-ci.com/github/python-pillow/pillow/settings.
@hugovk hugovk added the Release label Oct 23, 2023
@hugovk hugovk added this to the 10.2.0 milestone Oct 23, 2023
@hugovk
Copy link
Member Author

hugovk commented Nov 11, 2023

@aclark4life Please could you look into this? It needs to be done by someone with admin permissions. Thanks!

@aclark4life
Copy link
Member

@hugovk Sure, level of urgency?

@hugovk
Copy link
Member Author

hugovk commented Nov 13, 2023

It would be nice (but not essential) to have it ready for the next quarterly release.

@hugovk
Copy link
Member Author

hugovk commented Nov 27, 2023

Actually, we do need this before the next release, otherwise we'll need to do a similar hack to last time by uploading the files to a third-party site instead, and from a security perspective it would be better to avoid that.

@aclark4life
Copy link
Member

@hugovk OK I'll work on it this week

@aclark4life
Copy link
Member

aclark4life commented Nov 27, 2023

@hugovk When you say stuck do you mean this or something else? It's possible I need to make a better token for this task with the appropriate permissions … I have not done that yet.

Screenshot 2023-11-27 at 9 32 16 AM

@hugovk
Copy link
Member Author

hugovk commented Nov 27, 2023

Yes, I also got resource not found ({}: #7348 (comment)

@aclark4life
Copy link
Member

I cannot unsee what I've seen here today in working on this … but good news: progress!!

alexclark in ~/Developer/python-pillow/pillow on main λ travis accounts
aclark4life (Jeffrey A. Clark (Alex)): not subscribed, 4 repositories
plone (Plone Foundation): not subscribed, 392 repositories
python-pillow (Pillow): not subscribed, 5 repositories
To set up a subscription, please visit app.travis-ci.com.

@aclark4life
Copy link
Member

@hugovk OK more progress, although I'm not entirely sure why or how, I got repo commands working e.g.


alexclark on alexclarks-Air.fios-router.home in ~/Developer/python-pillow/pillow((default)|4h34m|travis-setup-releases*)
$ travis branches
10.1.0:                          #9957 errored    10.1.0 version bump
travis:                          #9952 passed     Test file change
master:                          #9929 passed     Merge pull request #5074 from hugovk/docs-links
aclark4life-patch-1:             #9922 passed     Update CHANGES.rst
8.0.x:                           #9830 passed     8.0.1 version bump
8.0.1:                           #9831 passed     8.0.1 version bump
8.0.0:                           #9800 passed     8.0.0 version bump
docs-python-versions-header:     #9652 passed     Give supported Python versions its own header
update-macos:                    #9442 passed     Tested 7.2.0 with 3.5-3.8 on macOS Mojave
autobuild-pr-test:               #9155 errored    Merge pull request #4663 from radarhere/lcms2
7.1.x:                           #9029 passed     7.1.2 version bump
7.1.2:                           #9028 passed     7.1.2 version bump
7.1.1:                           #8928 passed     Add 7.1.1 release notes to index
7.1.0:                           #8916 errored    7.1.0 version bump
7.2.0:                           #8914 passed     7.1.0 version bump
7.2.x:                           #8913 passed     7.1.0 version bump
coverage2:                       #8562 passed     macOS only
LAuk1obh6Lwn0eO28oEJh:           #8561 passed     Merge pull request #4357 from radarhere/testing
coverage:                        #8560 passed     macOS only
macos:                           #8558 passed     before
6.2.2:                           #8501 passed     Release notes for 6.2.2
7.0.0:                           #8500 passed     Release notes for 6.2.2
7.0.x:                           #8499 passed     Release notes for 6.2.2
6.2.x:                           #8497 passed     Release notes for 6.2.2
stringio:                        #8434 passed     Lint fix
rm-2.7-docs:                     #8325 passed     Update macOS to reflect GitHub Actions
rm-2.7-ci-docs:                  #8273 passed     Docs: remove 2.7 from CI targets
test-coverage5beta:              #8235 passed     Test Coverage 5 beta on the CIs
6.2.1:                           #8115 passed     Pillow 6.2.1 is the last to support Python 2.7
update-tidelift-badge:           #7891 passed     Update Tidelift badge
add-license-to-setup:            #7707 canceled   Add license to setup.py for #3938
6.1.0:                           #7698 passed     Revert #3780 for PyPy3 as it hasn't been updated yet. (#3935)
6.1.x:                           #7697 passed     Revert #3780 for PyPy3 as it hasn't been updated yet. (#3935)
revert-3780-update_py_unicode:   #7690 passed     Revert "Update Py_UNICODE to Py_UCS4"
5.2.x:                           #7683 failed     5.2.0 version bump
revert-3752-update-classifiers:  #7488 canceled   Revert "Update Trove licence classifiers"
6.0.0:                           #7306 passed     Resolved segfaults
6.0.x:                           #7305 passed     Resolved segfaults
segfault:                        #7303 passed     Resolved segfaults
update-readme:                   #7213 failed     Wording/linking
update-docs:                     #6709 passed     Release history is visible at PyPI
5.4.1:                           #6670 passed     5.4.0 version bump
5.4.x:                           #6672 passed     5.4.1 version bump
5.4.0:                           #6615 passed     Fix for check-manifest
update-release-notes:            #6603 passed     Fix typo
readthedocs:                     #6426 passed     Install project using pip in ReadTheDocs build
fix-webp-loading-from-blob:      #6399 passed     skip old test
5.3.0:                           #6372 passed     5.3.0 version bump
5.3.x:                           #6371 passed     5.3.0 version bump
fix-docstring-typo:              #6304 errored    Fix docstring typo

Unfortunately, travis setup releases is still complaining about GitHub tokens and it's really hard to tell what's going on, and what is supposed to happen, and when it's supposed to happen.

That said, I did manage to update Pillow's .travis.yml with an encrypted token. If you think that would help, I can commit it on a branch.

Screenshot 2023-11-28 at 12 03 32 PM

I ask because travis setup releases seems to want to overwrite .travis.yml before it fails on:

$ travis setup releases        
deploy section already exists in .travis.yml, run with --force to override
alexclark on alexclarks-Air.fios-router.home in ~/Developer/python-pillow/pillow((default)|4h40m|travis-setup-releases*)
$ travis setup releases --force
all GitHub tokens given were invalid

So maybe all it does is edit .travis.yml and we can skip that and just worry about the tokens ?

@hugovk
Copy link
Member Author

hugovk commented Nov 28, 2023

Okay, we might be nearly there... Rather than committing the token, save it in the Travis UI like this:

  1. Save the new token as GITHUB_RELEASE_TOKEN at app.travis-ci.com/github/python-pillow/pillow/settings.

@aclark4life
Copy link
Member

Done, although I didn't escape any of the characters in the secure token string …

@hugovk
Copy link
Member Author

hugovk commented Nov 28, 2023

Thanks! Now to figure out how to test it...

@radarhere
Copy link
Member

Is it reasonable to test it by just pushing a tag, letting Travis run and then deleting the tag afterwards?

@hugovk
Copy link
Member Author

hugovk commented Dec 4, 2023

Yeah, that's sounds good, it will check we trigger on tags properly.

Some people might be watching this repo for new tags and releases, so let's use something to make it clear it's a test.

@radarhere
Copy link
Member

Ok, I tried, and it "failed to deploy" - more specifically with an error that

/home/travis/.rvm/gems/ruby-3.1.2/gems/faraday-0.15.4/lib/faraday/options.rb:166:in `new': tried to create Proc object without a block (ArgumentError)

I found a suggestion to downgrade to focal, and if I reverted dea5bbe, then I made it to

GET https://api.github.com/user: 401 - Bad credentials // See: https://docs.github.com/rest (Octokit::Unauthorized)

This is an error that might be generated if the API key is wrong.

@aclark4life
Copy link
Member

In reviewing the syntax for the various configuration files, I noticed when encrypted tokens were listed, they were listed within a secure block e.g. instead of:

token: some-token

It was:

token:
  secure: some-encrypted-token

Or something like that. I'm not sure if that's an issue here, but it may be worth looking into and trying just to rule it out.

@radarhere
Copy link
Member

Instead of

Pillow/.travis.yml

Lines 43 to 46 in 0a66b98

# Upload wheels to GitHub Releases
deploy:
provider: releases
api_key: $GITHUB_RELEASE_TOKEN

I tried

# Upload wheels to GitHub Releases
deploy:
  provider: releases
  api_key:
    secure: $GITHUB_RELEASE_TOKEN

but to no avail.

@hugovk
Copy link
Member Author

hugovk commented Jan 2, 2024

Changing tack, we could cut out the middle steps:

  1. build wheel on Travis CI
  2. upload from Travis to GH release
  3. download locally
  4. upload to PyPI

And instead upload directly to PyPI from Travis:

  1. build wheel on Travis CI
  2. upload to PyPI

This would require creating an API token on PyPI and setting it as a TWINE_PASSWORD env var secret in the repo:

  -p PASSWORD, --password PASSWORD
                        The password to authenticate to the repository
                        (package index) with. (Can also be set via
                        TWINE_PASSWORD environment variable.)

https://twine.readthedocs.io

https://docs.travis-ci.com/user/environment-variables/#defining-variables-in-repository-settings

Then use twine upload on Travis CI.

We could try it with TestPyPI first.

Travis CI usually finishes before the GitHub Actions wheel build, so the Travis wheels will be uploaded before the sdist and other wheels. This should be fine.

What do you think?

@radarhere
Copy link
Member

Sure, sounds good if it works.

@hugovk
Copy link
Member Author

hugovk commented Jan 2, 2024

I've made an API token on TestPyPI, saved it in our 1Password vault ("Pillow TestPyPI API token"), and tried saving it as TWINE_PASSWORD at https://app.travis-ci.com/github/python-pillow/Pillow/settings but got an error:

image

Maybe it needs an admin to do it.

@aclark4life Please could you add it?

@hugovk hugovk modified the milestones: 10.2.0, 10.3.0 Jan 2, 2024
@aclark4life
Copy link
Member

It doesn't sound that great to me … but I may be missing something. (E.g. things that sound good to me: "No more Travis!") But let me take another close look over the next few days and report back, thanks all.

@hugovk
Copy link
Member Author

hugovk commented Jan 3, 2024

I was going to say:

"No more Travis!" sounds good to me too, and we can drop it, either by:

  • dropping the Linux aarch64 wheels
  • building them on GitHub Actions with emulation, but that would increase the build from ~40 mins to ~4 hours!

And now we have an automated release, maybe that's acceptable?

Then I decided to try it out, to see how long it takes now we have cibuildwheel.

First, Travis takes ~40 minutes for the longest job:

image

https://app.travis-ci.com/github/python-pillow/Pillow/builds/268090488

The first two creates manylinux wheels for 7 versions (3.8-3.12 + 2xPyPy) and the last musllinux for 5 versions (3.8-3.12).

The current GitHub Actions wheel build takes ~10 to ~40 minutes:

image

https://github.com/python-pillow/Pillow/actions/runs/7383669755

I added manylinux and musllinux to GitHub Actions using QEMU emulation. I forgot that Travis actually does two manylinux builds (manylinux_2_17 (aka manylinux2014) and manylinux_2_28), so we're actually missing some in this experiment, but let's come back to that.

I used a matrix, so each job ran for a single Python version and created 1 manylinux (by default manylinux_2_17) and 1 musllinx wheel (except PyPy which doesn't have musllinux so had a single manylinux).

The 7 QEMU jobs took 2-3 hours:

image

https://github.com/hugovk/Pillow/actions/runs/7387963337

Next, also split so each job does a single Python version and manylinux OR musllinux (except only manylinux for PyPy).

The 12 QEMU jobs took 1.5-2 hours:

image

https://github.com/hugovk/Pillow/actions/runs/7390943205

Is this an acceptable cost? We ditch Travis CI (40 minutes) and use GitHub Actions (2 hours)?

Note this the human waiting time, making use of parallel builds, the actually CPU time used is much greater.


As mentioned, I forgot manylinux_2_28 in this experiment.

Do we actually need wheels for both manylinux_2_17 and manylinux_2_28?

If so, this would add another 7 jobs to GHA, I expect also taking 1.5-2 hours.

We'd have 19 QEMU jobs taking 1.5-2 hours + 7 native jobs taking 10-40 mins. We get max 20 parallel jobs, so it would be fully saturated meaning some have to wait to be started, and so maybe take 2.5-3 hours for the whole thing.

@radarhere
Copy link
Member

I'm willing to deal with the additional time, sure.

Do we actually need wheels for both manylinux_2_17 and manylinux_2_28?

manylinux_2_28 was added in python-pillow/pillow-wheels#301, to allow for zlib 1.2.12 on newer operating systems, whereas manylinux_2_17 only allowed for zlib 1.2.8. The reason for keeping manylinux_2_17 around was to support CentOS 7 (EOL June 30) and Amazon Linux 2 (EOL June 30, 2025).

@wiredfool
Copy link
Member

AL2 is what we'd need for Lambda and Elastic Beanstalk.

@radarhere
Copy link
Member

Off-topic aside: I'm in favour of waiting until Amazon Linux 2 (and CentOS 7) are EOL - but there is a more recent AWS alternative for users in the form of Amazon Linux 2023.

@wiredfool
Copy link
Member

It looks like 3.8 and below are AL2, and 3.9+ are AL2023 for EB (https://docs.aws.amazon.com/elasticbeanstalk/latest/platforms/platform-history-python.html) , and 3.11 and below are AL2 for Lambda (https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtimes.html)

@hugovk
Copy link
Member Author

hugovk commented Jan 3, 2024

So, given CentOS 7 EOL is 2024-06-30, potentially we can forget about AL2 when Python 3.8 is EOL (2024-10-14), meaning we can drop manylinux_2_17 at the same time.

That would mean we have both manylinuxes for the next release in July, and just one for the following release in October.

@wiredfool
Copy link
Member

No, because Lambda is using AL2 for 3.11. And we certainly get enough questions about how to install on Lambda to know that it's something that people use.

@aclark4life
Copy link
Member

RIP AL2. Long live AL2023. (AL2 stops at Python 3.8. AL2023 supports up to 3.11)

@hugovk
Copy link
Member Author

hugovk commented Jan 3, 2024

Please see PR #7690 to replace Travis native builds with GHA emulated builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants