-
Notifications
You must be signed in to change notification settings - Fork 648
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 to PyPI via github action #247
Conversation
Replaced https://github.com/pypa/gh-action-pypi-publish with the twine invocation from the python-publish starter workflow. |
for d in opentelemetry-api/ opentelemetry-sdk/ ext/*/ ; do | ||
( | ||
cd "$d" | ||
python3 setup.py --verbose bdist_wheel --dist-dir "$BASEDIR/dist/" |
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.
Shouldn't we use pip wheel
instead of invoking setup.py
directly?
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.
seems like it just calls setup.py under the hood. Not sure why we'd want to abstract ourselves away a layer: https://pip.pypa.io/en/stable/reference/pip_wheel/#build-system-interface
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.
I couldn't find any guidance on whether pip wheel
or setup.py bdist_wheel
is preferred. I know that pip install
is preferred over setup.py install
but I guess the arguments there don't apply to pip wheel
. So I think it's fine either way.
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.
Looking at https://github.com/pypa/pip/blob/master/src/pip/_internal/commands/wheel.py, I think it's safe to say that pip wheel just calls setup.py is a gross oversimplification. But if we need all the extra work that pip does is a whole other question.
What would be useful is generating a source distribution first using setup.py and then generate a wheel from that using pip wheel
. That would verify that our MANIFEST.in, etc. is working correctly.
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.
I added #252 for building a source distribution first. I'll have to dig into the docs to develop an opinion here.
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.
One complication to note here is that pip wheel
isolates builds from the env.
setup.py bdist_wheel
builds as expected:
$ (cd opentelemetry-sdk/; python3 setup.py bdist_wheel --dist-dir='../dist' sdist --dist-dir='../dist'); ls dist
...
opentelemetry-sdk-0.4.dev0.tar.gz opentelemetry_sdk-0.4.dev0-py3-none-any.whl
pip wheel
fails because it tries to pip-install dependencies instead of using the current env:
(cd opentelemetry-sdk/; python3 setup.py sdist --dist-dir='../dist'); (cd dist; pip wheel opentelemetry-sdk-0.4.dev0.tar.gz); ls dist
...
Processing ./opentelemetry-sdk-0.4.dev0.tar.gz
ERROR: Could not find a version that satisfies the requirement opentelemetry-api==0.4.dev0 (from opentelemetry-sdk==0.4.dev0) (from versions: 0.1a0, 0.2a0, 0.3a0)
ERROR: No matching distribution found for opentelemetry-api==0.4.dev0 (from opentelemetry-sdk==0.4.dev0)
opentelemetry-sdk-0.4.dev0.tar.gz
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.
I have a couple relevant comments in #248 (sorry to comment on the WIP ticket), but no blockers. Nice call on automated releases!
@toumorokoshi's comment from the testing PR, https://github.com/open-telemetry/opentelemetry-python/pull/248/files#r340186555:
Don't universal wheels need to be compatible with python 2? Our releases are tagged |
…#247) * fix: rename FailedNoneRetryable -> FailedNotRetryable * fix: build
Fixes #8
This PR adds a GH action that builds all packages in the repo on each release and pushes them to PyPI.
The build script was inspired by OC's
twine_upload.sh
. This version only builds the wheels, and leaves it up to the GH action to publish to PyPI.I added a new PyPI API token for user cnk to this project's secrets.
See also #167, which added the first GH action to publish docs.