Skip to content
This repository was archived by the owner on Dec 8, 2021. It is now read-only.

Bazel-based codegen #3 (integrate protoc-java-resource-name-plugin)#38

Merged
vam-google merged 6 commits into
googleapis:masterfrom
vam-google:master
Sep 27, 2018
Merged

Bazel-based codegen #3 (integrate protoc-java-resource-name-plugin)#38
vam-google merged 6 commits into
googleapis:masterfrom
vam-google:master

Conversation

@vam-google

Copy link
Copy Markdown
Contributor
  1. Bazel version 0.16.0+ is required.
  2. Only building is now supported (tests are not executed from bazel).
  3. To reduce number of transitive dependencies gax was removed from dependencies.
    2.1) Gax-python is deprecated, so it is ok to remove deprecated dependency.
    2.2) Instead path_template.py from gax was directly copied to this repo (the only required thing from gax).
  4. Removed plugin_pb2.py. It was a hack from the beginning, the protobuf python whl packages problem (when they did not include plugin_pb2.py) are fixed in protobuf version 3.5.2+, so this hack is not needed anymore (also, since bazel builds everything from sources, even using older version of protobuf from bazel would not suffer from this issue).

1) Only building is now supported (tests are not executed from bazel)
2) To reduce number of transitive dependencies gax was removed from dependencies
    2.1) Gax-python is deprecated, so it is ok to remove deprecated dependency
    2.2) Instead path_template.py from gax was directly copied to this repo (the only neede thing from gax)
3) Removed plugin_pb2.py. It was a hack from the beginning, the protobuf python whl packages problem (when they did not include plugin_pb2.py) are fixed from version 3.5.2, so this hack is not neede anymore (also, since bazel run builds everything from sources, even using older version of protobuf from bazel would not suffer from this issue).
@michaelbausor

Copy link
Copy Markdown
Contributor

There is a version of path_template now distributed as part of the google-api-core package: https://github.com/GoogleCloudPlatform/google-cloud-python/blob/master/api_core/google/api_core/path_template.py

Should we depend on that instead of duplicating?

Comment thread WORKSPACE Outdated
"com_google_protoc_java_resource_names_plugin_repositories",
)

com_google_protoc_java_resource_names_plugin_repositories() No newline at end of file

This comment was marked as spam.

This comment was marked as spam.

Comment thread repositories.bzl Outdated
pypi_py_yaml()
if not omit_pypi_pystache:
pypi_pystache()
if not omit_pypi_six :

This comment was marked as spam.

This comment was marked as spam.

Comment thread repositories.bzl
name = "pypi_py_yaml",
url = ("https://files.pythonhosted.org/packages/4a/85/db5a2df477072b2902b0eb892feb37d88ac635d36245a72a6a69b23b383a/PyYAML-3.12.tar.gz"),
strip_prefix = "PyYAML-3.12/lib/yaml",
build_file_content = """

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Comment thread repositories.bzl Outdated
native.bind(
name = "six",
actual = "@pypi_six//:six"
) No newline at end of file

This comment was marked as spam.

@vam-google

vam-google commented Sep 20, 2018

Copy link
Copy Markdown
Contributor Author

@michaelbausor: path_template.py is distributed as part of google-api-core, but main reason of removing dependency on gax-python was reducing the transitive dependencies count (it is painful to import python deps in bazel). Taking into account that most probably this plugin will be rewritten in java and that there is no real need to depend on real python-core (we do not need the rest of python-core, and if core lib updates/changes it is very unlikely that we will need those updates here) it makes sense to just copy that single file in this repo. @garrettjonesgoogle what do you think?

@michaelbausor

Copy link
Copy Markdown
Contributor

@vam-google ok, that sounds fine to me

@garrettjonesgoogle

Copy link
Copy Markdown
Contributor

@vam-google sounds good to me (about copying path_template.py)

@vam-google

Copy link
Copy Markdown
Contributor Author

@garrettjonesgoogle PTAL

Comment thread requirements.txt Outdated
pystache>=0.5.4
protobuf>=3.3
google-gax>=0.12.3
protobuf>=3.6.0

This comment was marked as spam.

@garrettjonesgoogle garrettjonesgoogle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@vam-google
vam-google merged commit a1ad58a into googleapis:master Sep 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants