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 to PyPI via github action #247

Merged
merged 9 commits into from
Oct 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .github/workflows/publish.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Publish

on:
release:
types: [created]

jobs:
publish:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: actions/setup-python@v1
with:
python-version: '3.7'
- name: Build wheels
run: ./scripts/build.sh
- name: Publish to PyPI
env:
TWINE_USERNAME: '__token__'
TWINE_PASSWORD: ${{ secrets.pypi_password }}
run: |
pip install twine
twine upload --skip-existing --verbose dist/*
1 change: 1 addition & 0 deletions opentelemetry-api/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
description="OpenTelemetry Python API",
include_package_data=True,
long_description=open("README.rst").read(),
long_description_content_type="text/x-rst",
install_requires=["typing; python_version<'3.5'"],
extras_require={},
license="Apache-2.0",
Expand Down
1 change: 1 addition & 0 deletions opentelemetry-sdk/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
description="OpenTelemetry Python SDK",
include_package_data=True,
long_description=open("README.rst").read(),
long_description_content_type="text/x-rst",
install_requires=["opentelemetry-api==0.1.dev0"],
extras_require={},
license="Apache-2.0",
Expand Down
24 changes: 24 additions & 0 deletions scripts/build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/bin/sh

# This script builds wheels for the API, SDK, and extension packages in the
# dist/ dir, to be uploaded to PyPI.

set -ev

# Get the latest versions of packaging tools
python3 -m pip install --upgrade pip setuptools wheel

BASEDIR=$(dirname $(readlink -f $(dirname $0)))

(
cd $BASEDIR
mkdir -p dist
rm -rf dist/*

for d in opentelemetry-api/ opentelemetry-sdk/ ext/*/ ; do
(
cd "$d"
python3 setup.py --verbose bdist_wheel --dist-dir "$BASEDIR/dist/"
Copy link
Member

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?

Copy link
Member

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

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member Author

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

)
done
)