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

bazel(python): no py_(proto | grpc)_library targets to depend on when dependency has a GAPIC #46

Open
noahdietz opened this issue Jun 9, 2021 · 3 comments
Assignees
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Milestone

Comments

@noahdietz
Copy link
Contributor

When a proto depends on a proto in another package, there is a chance that the python target dependencies of the dependent target will be generated with the wrong dependency targets when the dependency is a GAPIC. This is because when a package is a GAPIC, the only python targets generated are the py_gapic_library and py_gapic_assembly_pkg targets - there are no py_proto_library or py_grpc_library targets generated. However, a dependent package is generated with py_proto_library and py_grpc_library targets as the dependencies. It really should be depending on the py_gapic_assembly_pkg target instead, which contains not only GAPIC code but also the protobuf & grpc code.

I think a good solution would be for the non-GAPIC packages to be genreated with a py_gapic_assembly_pkg target with just the grpc & protobuf targets as the packaged sources. This way, we can change build_gen to always generate a dependency on this single target rather than having to determine which targets a dependency has to use.

@noahdietz noahdietz added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Jun 9, 2021
@hkdevandla
Copy link
Member

@software-dov, can you please triage this?
@vam-google (for vis).

@hkdevandla
Copy link
Member

Note: This impacts only APIs with cross package dependencies and there are very few APIs that fall into this category. Still this needs to be fixed. Perhaps this could be part of the bazel robustification work.

@viacheslav-rostovtsev (for vis)

@noahdietz noahdietz added this to the build gen v2 milestone Oct 6, 2021
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Dec 6, 2021
@vchudnov-g vchudnov-g assigned atulep and unassigned software-dov Mar 15, 2022
@noahdietz noahdietz added priority: p3 Desirable enhancement or fix. May not be included in next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. labels May 9, 2022
@noahdietz
Copy link
Contributor Author

Downgrading to P3 as there aren't any urgent cases and this may not be feasible to do with existing infrastructure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

5 participants