Converts wheel storage from S3 to git-lfs #124
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
conorsch
force-pushed
the
wheels-via-git-lfs
branch
2 times, most recently
from
78d7fa0
to
3197e84
Compare
|
Seeing an odd CI failure only on WARNING: Retrying (Retry(total=4, connect=None, read=None, redirect=None, status=None)) after connection broken by 'SSLError(SSLError(1, '[SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:720)'),)': /simple/alembic/ WARNING: Retrying (Retry(total=3, connect=None, read=None, redirect=None, status=None)) after connection broken by 'SSLError(SSLError(1, '[SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:720)'),)': /simple/alembic/ WARNING: Retrying (Retry(total=2, connect=None, read=None, redirect=None, status=None)) after connection broken by 'SSLError(SSLError(1, '[SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:720)'),)': /simple/alembic/ WARNING: Retrying (Retry(total=1, connect=None, read=None, redirect=None, status=None)) after connection broken by 'SSLError(SSLError(1, '[SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:720)'),)': /simple/alembic/ WARNING: Retrying (Retry(total=0, connect=None, read=None, redirect=None, status=None)) after connection broken by 'SSLError(SSLError(1, '[SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:720)'),)': /simple/alembic/ Could not fetch URL https://pypi.securedrop.org/simple/alembic/: There was a problem confirming the ssl certificate: HTTPSConnectionPool(host='pypi.securedrop.org', port=443): Max retries exceeded with url: /simple/alembic/ (Caused by SSLError(SSLError(1, '[SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:720)'),)) - skipping Since all the other jobs are passing, I suspect it's a transient error. Will let it alone for a while so it can think about what it's done, then restart CI and see if it straightens up and flies right. |
|
we discussed removing the stretch jobs during standup today and all were in agreement, so once #126 is merged you can rebase and then the offending CI job will be gone |
added 2 commits
January 13, 2020 10:44
Pulled via the fetch-wheels operation. Removes gitignore settings, so we can commit these files (via git-lfs). Adds gitattributes settings, to track git-lfs config.
Updates documentation to remove mention of that step, since the wheels are now stored directly inside this repo.
conorsch
force-pushed
the
wheels-via-git-lfs
branch
from
3197e84
to
02bd7a5
Compare
added 2 commits
January 13, 2020 10:51
The old URL was:
https://dev-bin.ops.securedrop.org/localwheels
The new URL is:
https://pypi.securedrop.org/localwheels
There's an order-of-operations snag in that the new URL won't be updated
automatically until the new wheels are merged into the repo master.
Removes the fetch-wheels operation, which doesn't have a script anymore. Now we need to ensure that git-lfs is configured before building, in order to find the wheels locally (otherwise they'll just be pointers).
conorsch
force-pushed
the
wheels-via-git-lfs
branch
from
02bd7a5
to
9f51495
Compare
redshiftzero
approved these changes
Jan 13, 2020
.circleci/config.yml
Outdated
| @@ -273,8 +272,8 @@ jobs: | |||
| - image: circleci/python:3.7-buster | |||
| steps: | |||
| - checkout | |||
| - *installgitlfs | |||
There was a problem hiding this comment.
ah we should also either add a note to the readme that the dev should make sure they have git-lfs installed (alternatively we can add git-lfs package to the list in scripts/install-deps)
There was a problem hiding this comment.
Great call, we make the git-lfs recommendation in all the artifact repos, should absolutely do the same here. Will add!
There was a problem hiding this comment.
Do we just want to add the lfs to the install-deps action? Debian 10 has 2.7.1 in the dist repos, and the manual install logic in the CI config uses 2.7.2.
There was a problem hiding this comment.
yeah let's do that since debian has a packaged version
conorsch
force-pushed
the
wheels-via-git-lfs
branch
from
2223628
to
ae40671
Compare
Provides a convenient method for installing git-lfs, as part of the install-required-dependencies step. Added a conditional check to ensure that the wheels are indeed retrieved via git-lfs. This change is required by developers only on first-run, thus the conditional, but required in CI all the time, since the repo is cloned before deps are installed.
conorsch
force-pushed
the
wheels-via-git-lfs
branch
from
ae40671
to
deba9e7
Compare
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The binary wheels, and associated tarballs, previously stored in S3 buckets have now been moved into this repository, inside the
localwheels/dir, viagit-lfs.There's a significant drawback to this method: the public URL (https://pypi.securedrop.org/simple/) is only updated upon merge into the
masterbranch of this repository. In order to test locally, e.g. during preparation of packaging changes, or during review of an unmerged PR, the docs now recommend usingpython3 -m http.serverto spin up a local server, and editing the index URL in-place. That's a bit clunky, but with that small workflow change, we can ditch S3 entirely, in favor of the wonderfully more auditable git-lfs.Closes #109
Practically speaking, we should review and merge #112 first, before these changes, in order to reduce conflicts.
Testing
Make sure you have git-lfs installed and configured!
Then, read through the documentation changes to make sure the new workflow is clear. Then try building a package. This is what I tried:
PKG_VERSION=0.0.11 make securedrop-clientThat will pull from the public URL. Use the
python3 -m http.serverapproach to test the local fetching, e.g. by bumping a pip dep and ensuring the new wheel can be found during package build.Post-merge, we'll need to update the PyPI URL branch-tracking logic to track "master" (it's currently tracking this branch, to aid in review).