-
Notifications
You must be signed in to change notification settings - Fork 69
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
feat: Initial draft of GAPIC Bazel Extensions gapic-generator-python #342
Conversation
Codecov Report
@@ Coverage Diff @@
## master #342 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 25 25
Lines 1397 1397
Branches 289 289
=====================================
Hits 1397 1397 Continue to review full report at Codecov.
|
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.
Looks good so far, just a few questions.
requirement("pypandoc"), | ||
requirement("PyYAML"), | ||
], | ||
python_version = "PY3", |
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.
Is there any way we can be more specific about the python version requirement? Some of the code in the generator requires features from >=3.6
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.
There are ways to be more specific, but they have their own issues (i.e. make the build less hermetic and less portable). Practically speaking, it should be picking the latest installed version of python on the host machine, and if there is 3.6+ bazel will pick it up.
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.
Okay. Good to know.
rules_python_gapic/py_gapic.bzl
Outdated
@@ -0,0 +1,18 @@ | |||
load("@com_google_api_codegen//rules_gapic:gapic.bzl", "proto_custom_library") | |||
|
|||
def py_gapic_library2(name, srcs, **kwargs): |
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.
Can we pass custom options through **kwargs? Is it straightforward?
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.
yes we can, we can also always add those custom options explicitly as args in this rule. It is straightforward to do (I did not add any because of my lack of knowledge and context 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.
Excellent. We will try using kwargs first and see if explicit args are necessary.
@software-dov I cleaned up this PR, and added instructions how it can be used from googleapis to test stuff, PTAL. |
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.
LGTM. One question: I don't see a hash anywhere that corresponds to gapic-generator-python; other things yes, but not that one. Does the rule automatically use the remote repository master? Do I need to update anything if I cut a new release?
@software-dov Maybe there is some misunderstanding how this things (bazel rules and bazel workspaces) work together. There is a workspace which defines a rule (this one, gapic-generator-python defines the http_archive(
name = "gapic_generator_python",
urls = ["https://github.com/vam-google/gapic-generator-python/archive/77637d9bfdbfcb67b02b3d6bbbc6938de44214ce.zip"],
strip_prefix = "gapic-generator-python-77637d9bfdbfcb67b02b3d6bbbc6938de44214ce",
) Where |
@software-dov Pleas push this PR if it looks good to you (i don't have rights). |
Okay, so if I do cut new releases (or make new commits on top of master) I need to change the hash in googleapis' workspace file. Got it. |
This is a "minimal viable product" version of the rule, which simply call the python microgenerator to generate a client. No post and/or preprocessing is implemented, but the actual GAPIC code is produced successfully.
Successfully tested on
googleapis/google/cloud/lanugage/v1
API.To use it from googleapis, add the following to the
# Python
in WORKSPACE file in googleapis:and the following at the top of the file (needs to go to the top, because
rules_python
of older version is a transitive dependency ofgrpc
rules, we need a newer version to be used in workspace, so specify it first to override the older one).The in
google/cloud/langugae/v1/BUILD.bazel
file in Python section addand build it with: