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

Use Python namespace packages #6811

Merged
merged 1 commit into from
Jun 9, 2016
Merged

Use Python namespace packages #6811

merged 1 commit into from
Jun 9, 2016

Conversation

soltanmm
Copy link
Contributor

@soltanmm soltanmm commented Jun 7, 2016

This is definitively needed before GA, lest we ship packages that cause users the duress of having to invoke installation commands twice.

Made a PR for spot-check review. Still need to check the packaged wheels.

cc @nathanielmanistaatgoogle @kpayson64

@soltanmm
Copy link
Contributor Author

soltanmm commented Jun 7, 2016

The 📦s check out fine on my machine (well, the ones that are built to work on my machine, anyway).

@soltanmm
Copy link
Contributor Author

soltanmm commented Jun 7, 2016

@nathanielmanistaatgoogle
Copy link
Member

Looks reasonable, but I'm not entirely sure what I'm seeing. Any desire to embellish the commit message?

Before, namespace packages would break in auditwheel due to a
superfluous check (fixed in the commit referenced in the Dockerfiles).
Now, the auditwheel used in the manylinux1-support Docker images can
handle namespace packages, and we may thus use them. This should
alleviate future user pain w.r.t. installation of grpcio-tools causing
loss of grpcio files and vice versa (e.g. requiring a reinstall of
grpcio following installation of grpcio-tools).
@soltanmm
Copy link
Contributor Author

soltanmm commented Jun 7, 2016

@nathanielmanistaatgoogle Embellished.

@hsaliak hsaliak modified the milestones: 2016/05/21 Milestone, GA Jun 7, 2016
@kpayson64
Copy link
Contributor

Looks reasonable, I'd just ask that we cut a low priority issue to install auditwheel from pip when they cut their next release.

@soltanmm
Copy link
Contributor Author

soltanmm commented Jun 8, 2016

@kpayson64 Yes; I'd prefer to file that issue following this PR going in to avoid dangling references, as the version of auditwheel that we use is the default one that goes into the Docker image distributed by the PyPA folks, which is the latest release at the time of the docker image. I can promise that the issue will read roughly, "Remove the Dockerfile commands added in #6811 following auditwheel's next release," though.

@nathanielmanistaatgoogle
Copy link
Member

Luhguhtum. Is this ready for merge?

@jtattermusch jtattermusch merged commit c266368 into grpc:master Jun 9, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants