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

Add __init__.py to directories created by grpcio #7984

Merged
merged 9 commits into from Jul 4, 2019

Conversation

revl
Copy link
Contributor

@revl revl commented Jun 30, 2019

Problem

When trying to create a dist tarball from python_grpcio_library using provides=setup_py, the find-namespace-packages step fails with the following errors:

$ ./pants setup-py ...
[...]
..._pb2.py is source but does not belong to a package.
..._pb2_grpc.py is source but does not belong to a package.

That happens because the find_packages() method in setup_py.py ignores subdirectories that do not contain __init__.py.

Solution

This commit adds an empty __init__.py file to each directory created by grpcio.

Result

A package with gRPC interfaces is properly created.

### Problem

When trying to create a dist tarball from `python_grpcio_library`
using `provides=setup_py`, the `find-namespace-packages` step fails
with the following errors:

    $ ./pants setup-py ...
    [...]
    ..._pb2.py is source but does not belong to a package.
    ..._pb2_grpc.py is source but does not belong to a package.

That happens because the `find_packages()` method in `setup_py.py`
ignores subdirectories that do not contain `__init__.py`.

### Solution

This commit adds an empty `__init__.py` file to each directory
created by grpcio.

### Result

A package with gRPC interfaces is properly created.
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for the contribution! The overall fix looks great (thanks for updating the tests too) - provided some ways to tie in with the rest of the codebase better.

@Eric-Arellano
Copy link
Contributor

Also looks like the lint shard failed due to import order. One tip: run build-support/bin/setup.sh to install githooks that will run before making any commits.

In this particular instance, you can fix the import error by running build-support/bin/isort.sh -f.

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for iterating on this!

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Ran this locally and confirmed the tests work for me. Was trying to see if we can workaround relative_to but you're right that we need it.

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Will merge this upon green CI. We have a few flaky tests, so if it’s red at first don’t worry that you caused it - I’ll monitor and restart.

Have a great 4th of July :)

@revl
Copy link
Contributor Author

revl commented Jul 4, 2019

Thank you, Eric! You too have a great 4th of July! :-)

@Eric-Arellano Eric-Arellano merged commit 145fbf9 into pantsbuild:master Jul 4, 2019
@revl revl deleted the add-init-py-to-grpcio-output branch July 5, 2019 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants