Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

chore(spanner): Add benchwrapper.#657

Merged
ansh0l merged 11 commits intogoogleapis:mainfrom
rajatbhatta:spanner-client-libs-benchmarking
Jan 7, 2022
Merged

chore(spanner): Add benchwrapper.#657
ansh0l merged 11 commits intogoogleapis:mainfrom
rajatbhatta:spanner-client-libs-benchmarking

Conversation

@rajatbhatta
Copy link
Copy Markdown
Contributor

@rajatbhatta rajatbhatta commented Jan 3, 2022

This PR adds a Benchwrapper implementation for Python Spanner Client Library.

The benchwrapper implementation contains the following:

  • The proto file, as well as the generated classes from the proto file.
  • A file that connects to the emulator using the SPANNER_EMULATOR_HOST environment variable, and implements the RPC’s defined in the proto file.

Checks:

  • Appropriate docs were added, wherever necessary.
  • Ensure the tests and linter pass.

@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner API. label Jan 3, 2022
@zoercai zoercai marked this pull request as ready for review January 3, 2022 23:09
@zoercai zoercai requested a review from a team January 3, 2022 23:09
@zoercai zoercai requested a review from a team as a code owner January 3, 2022 23:09
Copy link
Copy Markdown
Contributor

@zoercai zoercai left a comment

Choose a reason for hiding this comment

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

Please also get an approval from a Python reviewer before merging, my LGTM is only for what's been implemented, not the Python syntax/readability.

@vi3k6i5 vi3k6i5 self-requested a review January 4, 2022 09:45
Copy link
Copy Markdown

@IlyaFaer IlyaFaer left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@vi3k6i5 vi3k6i5 left a comment

Choose a reason for hiding this comment

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

LGTM

-   Add script docstring with usage instructions.

-   Modify copyright in spanner.proto.
@ansh0l
Copy link
Copy Markdown
Contributor

ansh0l commented Jan 6, 2022

This branch is out-of-date with the base branch - please update before merging

@rajatbhatta
Copy link
Copy Markdown
Contributor Author

This branch is out-of-date with the base branch - please update before merging

@ansh0l : This is done. Please merge it. Thank you!

Copy link
Copy Markdown
Contributor

@ansh0l ansh0l left a comment

Choose a reason for hiding this comment

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

LGTM.

Minor: Please check if we can name these files (_pb2) better

benchmark/benchwrapper/proto/spanner_pb2.py
benchmark/benchwrapper/proto/spanner_pb2_grpc.py

@ansh0l ansh0l merged commit 1c4aeca into googleapis:main Jan 7, 2022
@rajatbhatta
Copy link
Copy Markdown
Contributor Author

LGTM.

Minor: Please check if we can name these files (_pb2) better

benchmark/benchwrapper/proto/spanner_pb2.py benchmark/benchwrapper/proto/spanner_pb2_grpc.py

@ansh0l : These were not written manually but were generated using grpc tool, and the so the files should not be renamed, or changed in any way.

Thank you for merging the PR!

vi3k6i5 pushed a commit to vi3k6i5/python-spanner that referenced this pull request Mar 29, 2022
* chore(spanner): Add benchwrapper.

* Add README.md to run the benchwrapper

* Add README for regenerating protos for benchmarking

* Small change to benchwrapper README

* Update README.md for benchwrapper.

* chore(spanner): Incorporate review comments.

* chore(spanner): Incorporate review comments.

* chore(spanner): accommodate review comments.

-   Add script docstring with usage instructions.

-   Modify copyright in spanner.proto.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: spanner Issues related to the googleapis/python-spanner API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants