Skip to content

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented Dec 22, 2015

@tseaver As you'll notice, this is quite nasty. I'm open to suggestions for making this better. Some simple options:

  • We are content that this is "good enough"
  • I become competent using bash
  • I rewrite this script in Python with judicious use of some system calls

FWIW, this is in advance of v1beta3 for datastore, which will re-use some of these components (e.g. timestamp) so we need this pipeline to be a bit more polished.

@dhermes dhermes added the api: bigtable Issues related to the Bigtable API. label Dec 22, 2015
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 22, 2015

This comment was marked as spam.

@dhermes dhermes mentioned this pull request Dec 22, 2015
49 tasks
@tseaver
Copy link
Contributor

tseaver commented Dec 22, 2015

It seems a little weird that this is a makefile, given that the logic is all imperative. It seems like there should be a way to use pattern-based dependency rules to do the compilation / copying.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 22, 2015

I agree, I hate it. Should I just take a stab at re-writing this in Python (where I can actually use a for loop)?


I must say I don't know much about the protoc tool.

  • There is a way to specify --proto_path to look for dependencies, but we end up having lots of paths
  • There is no way in protoc to accomplish the goal of this PR (copying over the .proto files while also renaming them with an _ in front)
  • There is a flag that may be useful that is in the updated protoc but not in the one in the Debian package repo
--dependency_out=FILE       Write a dependency output file in the format
                            expected by make. This writes the transitive
                            set of input file paths to FILE
  • Since the generated files all live in the google/... file structure and the libraries don't use a namespace package, we have to move them, so again we have to manually locate the path
  • It seems that the following is supported
protoc --proto_path=src --python_out=build/gen src/foo.proto src/bar/baz.proto

which produces build/gen/foo_pb2.py and build/gen/bar/baz_pb2.py

@dhermes dhermes force-pushed the makefile-bring-over-dot-proto branch from bde00aa to 7a09564 Compare December 22, 2015 23:06
@dhermes
Copy link
Contributor Author

dhermes commented Dec 22, 2015

@tseaver PTAL. This is (hopefully) much easier to read and for now can hold off implementing in Python (and maybe for always).

@dhermes dhermes force-pushed the makefile-bring-over-dot-proto branch from 7a09564 to fd2bfe2 Compare January 4, 2016 04:42
@dhermes dhermes force-pushed the makefile-bring-over-dot-proto branch from fd2bfe2 to 5be7afc Compare January 7, 2016 23:45
@tseaver
Copy link
Contributor

tseaver commented Jan 14, 2016

Rather than treating the makefile like a bash script, could we make the generated _pb2.py files in the right locations be actual make targets, and then write pattern-based rules for producing them (in the two or three steps needed) from the original .proto files?

@dhermes
Copy link
Contributor Author

dhermes commented Jan 14, 2016

Want to take a stab at it?

Fully revamping the script to be more read-able, split
into sections:

- Compile `.proto` to `_pb2.py` files
- Move over `_pb2.py` files into our library
- Clear out old `.proto` files in library
- Move over the newly compiled `.proto` files into library
  and remove the executable bits and prepend them with `_`
@dhermes dhermes force-pushed the makefile-bring-over-dot-proto branch from 5be7afc to d97004a Compare January 14, 2016 21:31
@dhermes
Copy link
Contributor Author

dhermes commented Jan 14, 2016

@tseaver can we punt on the Makefile optimizations and try to handle it if/when the service specific protos get their own repo?

The point of this PR was to prepare for building v1beta3 in a more systematic way but it is blocking other PRs.

If you'd like I could also pull the relevant content apart from PRs that use this as a diff base

@tseaver
Copy link
Contributor

tseaver commented Jan 14, 2016

@dhermes If datastore v1beta3 is the driver, I'm not sure why we're mucking about with generating protobuf stuff: wouldn't we be better off just importing from googleapis-common-protos?

@dhermes
Copy link
Contributor Author

dhermes commented Jan 14, 2016

That will only contain "common" protos (see #1353 for which protos that means). This doesn't help for service specific protos (Bigtable, datastore and soon pub/sub and logging)

@tseaver
Copy link
Contributor

tseaver commented Jan 14, 2016

Weird. The googleapis repository actually contains the datastore v1beta3 protos. It doesn't have any of the code for making the Python releases, however.

@dhermes
Copy link
Contributor Author

dhermes commented Jan 14, 2016

@tbetbetbe never told me how they generated the code but the point of common is the set of protos that are utilities used by services but aren't defined within a specific service (e.g. google.api.duration)

@tseaver
Copy link
Contributor

tseaver commented Jan 14, 2016

If we could get the googleapis team to either a) make PyPI releases for the service-specific protobuf wrappers (`googleapis-datastore-protos', etc.), or b) publish how they are generating the one they made already, that would be helpful. I'm currently hacking on how to use their Makefile to generate the Python code.

@dhermes
Copy link
Contributor Author

dhermes commented Jan 14, 2016

@tseaver I filed #1384 to track this and cc-ed the lead of the team in charge PyPI package for common. Can we merge this and discuss the path forward in the issue?

@tseaver
Copy link
Contributor

tseaver commented Jan 14, 2016

Yup. LGTM

@dhermes
Copy link
Contributor Author

dhermes commented Jan 14, 2016

FWIW I've suggested your a) in recent conversations with Tim.

dhermes added a commit that referenced this pull request Jan 14, 2016
Making pb2 auto-gen script also copy over .proto files.
@dhermes dhermes merged commit 533b3c6 into googleapis:master Jan 14, 2016
@dhermes dhermes deleted the makefile-bring-over-dot-proto branch January 14, 2016 22:22
parthea pushed a commit that referenced this pull request Nov 24, 2025
* Add Error Reporting Code

* Updating doc links [(#324)](GoogleCloudPlatform/python-docs-samples#324)

* Commenting noxfile, renaming a test file.

Change-Id: Ice25caa7c035c24a585a66575dda69e170862df8

* Update requirements. [(#436)](GoogleCloudPlatform/python-docs-samples#436)

* Fix import order lint errors

Change-Id: Ieaf7237fc6f925daec46a07d2e81a452b841198a

* bump

Change-Id: I02e7767d13ba267ee9fc72c5b68a57013bb8b8d3

* Auto-update dependencies. [(#537)](GoogleCloudPlatform/python-docs-samples#537)

* Update samples to support latest Google Cloud Python [(#656)](GoogleCloudPlatform/python-docs-samples#656)

* Switch Error Reporting to Google Cloud Python [(#663)](GoogleCloudPlatform/python-docs-samples#663)

* Switch Error Reporting to Google Cloud Python

* Switch Error Reporting to Google Cloud Python

* Auto-update dependencies. [(#715)](GoogleCloudPlatform/python-docs-samples#715)

* Auto-update dependencies. [(#914)](GoogleCloudPlatform/python-docs-samples#914)

* Auto-update dependencies.

* xfail the error reporting test

* Fix lint

* Re-generate all readmes

* Auto-update dependencies. [(#922)](GoogleCloudPlatform/python-docs-samples#922)

* Auto-update dependencies.

* Fix pubsub iam samples

* Fix README rst links [(#962)](GoogleCloudPlatform/python-docs-samples#962)

* Fix README rst links

* Update all READMEs

* Auto-update dependencies. [(#1004)](GoogleCloudPlatform/python-docs-samples#1004)

* Auto-update dependencies.

* Fix natural language samples

* Fix pubsub iam samples

* Fix language samples

* Fix bigquery samples

* Auto-update dependencies. [(#1011)](GoogleCloudPlatform/python-docs-samples#1011)

* Auto-update dependencies. [(#1055)](GoogleCloudPlatform/python-docs-samples#1055)

* Auto-update dependencies.

* Explicitly use latest bigtable client

Change-Id: Id71e9e768f020730e4ca9514a0d7ebaa794e7d9e

* Revert language update for now

Change-Id: I8867f154e9a5aae00d0047c9caf880e5e8f50c53

* Remove pdb. smh

Change-Id: I5ff905fadc026eebbcd45512d4e76e003e3b2b43

* Auto-update dependencies. [(#1093)](GoogleCloudPlatform/python-docs-samples#1093)

* Auto-update dependencies.

* Fix storage notification poll sample

Change-Id: I6afbc79d15e050531555e4c8e51066996717a0f3

* Fix spanner samples

Change-Id: I40069222c60d57e8f3d3878167591af9130895cb

* Drop coverage because it's not useful

Change-Id: Iae399a7083d7866c3c7b9162d0de244fbff8b522

* Try again to fix flaky logging test

Change-Id: I6225c074701970c17c426677ef1935bb6d7e36b4

* Update all generated readme auth instructions [(#1121)](GoogleCloudPlatform/python-docs-samples#1121)

Change-Id: I03b5eaef8b17ac3dc3c0339fd2c7447bd3e11bd2

* Added Link to Python Setup Guide [(#1158)](GoogleCloudPlatform/python-docs-samples#1158)

* Update Readme.rst to add Python setup guide

As requested in b/64770713.

This sample is linked in documentation https://cloud.google.com/bigtable/docs/scaling, and it would make more sense to update the guide here than in the documentation.

* Update README.rst

* Update README.rst

* Update README.rst

* Update README.rst

* Update README.rst

* Update install_deps.tmpl.rst

* Updated readmegen scripts and re-generated related README files

* Fixed the lint error

* Auto-update dependencies. [(#1186)](GoogleCloudPlatform/python-docs-samples#1186)

* Add error reporting sample for manual reporting

* Readd fluentd samples

* rename dir

* Auto-update dependencies. [(#1205)](GoogleCloudPlatform/python-docs-samples#1205)

* Auto-update dependencies. [(#1215)](GoogleCloudPlatform/python-docs-samples#1215)

* Auto-update dependencies. [(#1245)](GoogleCloudPlatform/python-docs-samples#1245)

* Auto-update dependencies. [(#1248)](GoogleCloudPlatform/python-docs-samples#1248)

* Added "Open in Cloud Shell" buttons to README files [(#1254)](GoogleCloudPlatform/python-docs-samples#1254)

* Auto-update dependencies. [(#1276)](GoogleCloudPlatform/python-docs-samples#1276)

* Auto-update dependencies. [(#1282)](GoogleCloudPlatform/python-docs-samples#1282)

* Auto-update dependencies.

* Fix storage acl sample

Change-Id: I413bea899fdde4c4859e4070a9da25845b81f7cf

* Auto-update dependencies. [(#1317)](GoogleCloudPlatform/python-docs-samples#1317)

* Auto-update dependencies. [(#1320)](GoogleCloudPlatform/python-docs-samples#1320)

* Auto-update dependencies. [(#1359)](GoogleCloudPlatform/python-docs-samples#1359)

* Auto-update dependencies. [(#1377)](GoogleCloudPlatform/python-docs-samples#1377)

* Auto-update dependencies.

* Update requirements.txt

* Regenerate the README files and fix the Open in Cloud Shell link for some samples [(#1441)](GoogleCloudPlatform/python-docs-samples#1441)

* Update READMEs to fix numbering and add git clone [(#1464)](GoogleCloudPlatform/python-docs-samples#1464)

* Auto-update dependencies. [(#1658)](GoogleCloudPlatform/python-docs-samples#1658)

* Auto-update dependencies.

* Rollback appengine/standard/bigquery/.

* Rollback appengine/standard/iap/.

* Rollback bigtable/metricscaler.

* Rolledback appengine/flexible/datastore.

* Rollback dataproc/

* Rollback jobs/api_client

* Rollback vision/cloud-client.

* Rollback functions/ocr/app.

* Rollback iot/api-client/end_to_end_example.

* Rollback storage/cloud-client.

* Rollback kms/api-client.

* Rollback dlp/

* Rollback bigquery/cloud-client.

* Rollback iot/api-client/manager.

* Rollback appengine/flexible/cloudsql_postgresql.

* Put in new region code error_reporting_quickstart [(#1842)](GoogleCloudPlatform/python-docs-samples#1842)

* New region tags [(#1843)](GoogleCloudPlatform/python-docs-samples#1843)

* Auto-update dependencies. [(#1980)](GoogleCloudPlatform/python-docs-samples#1980)

* Auto-update dependencies.

* Update requirements.txt

* Update requirements.txt

* Adds updates including compute [(#2436)](GoogleCloudPlatform/python-docs-samples#2436)

* Adds updates including compute

* Python 2 compat pytest

* Fixing weird \r\n issue from GH merge

* Put asset tests back in

* Re-add pod operator test

* Hack parameter for k8s pod operator

* Auto-update dependencies. [(#2005)](GoogleCloudPlatform/python-docs-samples#2005)

* Auto-update dependencies.

* Revert update of appengine/flexible/datastore.

* revert update of appengine/flexible/scipy

* revert update of bigquery/bqml

* revert update of bigquery/cloud-client

* revert update of bigquery/datalab-migration

* revert update of bigtable/quickstart

* revert update of compute/api

* revert update of container_registry/container_analysis

* revert update of dataflow/run_template

* revert update of datastore/cloud-ndb

* revert update of dialogflow/cloud-client

* revert update of dlp

* revert update of functions/imagemagick

* revert update of functions/ocr/app

* revert update of healthcare/api-client/fhir

* revert update of iam/api-client

* revert update of iot/api-client/gcs_file_to_device

* revert update of iot/api-client/mqtt_example

* revert update of language/automl

* revert update of run/image-processing

* revert update of vision/automl

* revert update testing/requirements.txt

* revert update of vision/cloud-client/detect

* revert update of vision/cloud-client/product_search

* revert update of jobs/v2/api_client

* revert update of jobs/v3/api_client

* revert update of opencensus

* revert update of translate/cloud-client

* revert update to speech/cloud-client

Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com>
Co-authored-by: Doug Mahugh <dmahugh@gmail.com>

* chore(deps): update dependency fluent-logger to v0.9.5 [(#3004)](GoogleCloudPlatform/python-docs-samples#3004)

* chore(deps): update dependency fluent-logger to v0.9.6 [(#3080)](GoogleCloudPlatform/python-docs-samples#3080)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [fluent-logger](https://togithub.com/fluent/fluent-logger-python) | patch | `==0.9.5` -> `==0.9.6` |

---

### Release Notes

<details>
<summary>fluent/fluent-logger-python</summary>

### [`v0.9.6`](https://togithub.com/fluent/fluent-logger-python/compare/v0.9.5...v0.9.6)

[Compare Source](https://togithub.com/fluent/fluent-logger-python/compare/v0.9.5...v0.9.6)

</details>

---

### Renovate configuration

:date: **Schedule**: At any time (no schedule defined).

:vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

:recycle: **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

:no_bell: **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#GoogleCloudPlatform/python-docs-samples).

* Simplify noxfile setup. [(#2806)](GoogleCloudPlatform/python-docs-samples#2806)

* chore(deps): update dependency requests to v2.23.0

* Simplify noxfile and add version control.

* Configure appengine/standard to only test Python 2.7.

* Update Kokokro configs to match noxfile.

* Add requirements-test to each folder.

* Remove Py2 versions from everything execept appengine/standard.

* Remove conftest.py.

* Remove appengine/standard/conftest.py

* Remove 'no-sucess-flaky-report' from pytest.ini.

* Add GAE SDK back to appengine/standard tests.

* Fix typo.

* Roll pytest to python 2 version.

* Add a bunch of testing requirements.

* Remove typo.

* Add appengine lib directory back in.

* Add some additional requirements.

* Fix issue with flake8 args.

* Even more requirements.

* Readd appengine conftest.py.

* Add a few more requirements.

* Even more Appengine requirements.

* Add webtest for appengine/standard/mailgun.

* Add some additional requirements.

* Add workaround for issue with mailjet-rest.

* Add responses for appengine/standard/mailjet.

Co-authored-by: Renovate Bot <bot@renovateapp.com>

* chore(deps): update dependency google-cloud-error-reporting to v0.34.0 [(#3782)](GoogleCloudPlatform/python-docs-samples#3782)

* chore(deps): update dependency pytest to v5.4.3 [(#4279)](GoogleCloudPlatform/python-docs-samples#4279)

* chore(deps): update dependency pytest to v5.4.3

* specify pytest for python 2 in appengine

Co-authored-by: Leah Cole <coleleah@google.com>

* chore(deps): update dependency mock to v4 [(#4287)](GoogleCloudPlatform/python-docs-samples#4287)

* chore(deps): update dependency mock to v4

* specify mock version for appengine python 2

Co-authored-by: Leah Cole <coleleah@google.com>

* Update dependency pytest to v6 [(#4390)](GoogleCloudPlatform/python-docs-samples#4390)

* chore: update templates

Co-authored-by: Bill Prin <waprin@google.com>
Co-authored-by: Jon Wayne Parrott <jonwayne@google.com>
Co-authored-by: DPE bot <dpebot@google.com>
Co-authored-by: Bill Prin <waprin@gmail.com>
Co-authored-by: michaelawyu <chenyumic@google.com>
Co-authored-by: Andrew Gorcester <gorcester@google.com>
Co-authored-by: Andrew Gorcester <andrew.gorcester@gmail.com>
Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>
Co-authored-by: Charles Engelke <github@engelke.com>
Co-authored-by: Gus Class <gguuss@gmail.com>
Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com>
Co-authored-by: Doug Mahugh <dmahugh@gmail.com>
Co-authored-by: WhiteSource Renovate <bot@renovateapp.com>
Co-authored-by: Leah Cole <coleleah@google.com>
parthea pushed a commit that referenced this pull request Nov 26, 2025
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants