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

feat: add RPC priority support #324

Merged
merged 9 commits into from Jun 22, 2021
Merged

feat: add RPC priority support #324

merged 9 commits into from Jun 22, 2021

Conversation

zoercai
Copy link
Contributor

@zoercai zoercai commented Apr 28, 2021

Add client library support to enable users to optionally specify a priority level on their requests. All requests are run as PRIORITY_HIGH by default, this allows tasks to be deprioritised.

@zoercai zoercai requested a review from larkee Apr 28, 2021
@zoercai zoercai requested a review from as a code owner Apr 28, 2021
@product-auto-label product-auto-label bot added the api: spanner label Apr 28, 2021
@google-cla google-cla bot added the cla: yes label Apr 28, 2021
Copy link
Contributor

@larkee larkee left a comment

LG overall however there is some functionality missing.

First, all commit methods should support this, including Batch.commit().
Secondly, we usually call commit on behalf of the user which means they will not be able to pass the options to the commit call themselves. This means we need to provide alternate ways of passing in the options. e.g.

database.run_in_transaction(transaction_func, request_options={"priority": "PRIORITY_LOW"})
for database.batch(request_options={"priority": "PRIORITY_MEDIUM"}) as batch:
    batch.insert(table, columns, data)

Additionally, you should update the PR description to give context on the feature and how it should be used. For example, it should mention that all requests are run as PRIORITY_HIGH by default and so this feature is to deprioritize tasks.

tests/unit/test_database.py Outdated Show resolved Hide resolved
tests/system/test_system.py Outdated Show resolved Hide resolved
@zoercai zoercai requested a review from larkee May 3, 2021
google/cloud/spanner_v1/database.py Show resolved Hide resolved
google/cloud/spanner_v1/session.py Show resolved Hide resolved
google/cloud/spanner_v1/database.py Outdated Show resolved Hide resolved
google/cloud/spanner_v1/batch.py Outdated Show resolved Hide resolved
@zoercai zoercai requested a review from larkee Jun 8, 2021
Copy link
Contributor

@larkee larkee left a comment

Some minor docstring nits :)

google/cloud/spanner_v1/database.py Outdated Show resolved Hide resolved
google/cloud/spanner_v1/database.py Outdated Show resolved Hide resolved
google/cloud/spanner_v1/session.py Outdated Show resolved Hide resolved
zoercai and others added 2 commits Jun 11, 2021
Co-authored-by: larkee <31196561+larkee@users.noreply.github.com>
Co-authored-by: larkee <31196561+larkee@users.noreply.github.com>
@snippet-bot
Copy link

@snippet-bot snippet-bot bot commented Jun 11, 2021

No region tags are edited in this PR.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@zoercai zoercai requested a review from larkee Jun 11, 2021
larkee
larkee approved these changes Jun 11, 2021
@larkee larkee added the kokoro:force-run label Jun 11, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Jun 11, 2021
@larkee larkee added kokoro:force-run and removed kokoro:force-run labels Jun 11, 2021
@larkee larkee added the kokoro:force-run label Jun 15, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Jun 15, 2021
zoercai added 2 commits Jun 22, 2021
… rpc-priority

# Conflicts:
#	google/cloud/spanner_v1/__init__.py
#	tests/unit/test_transaction.py
@larkee larkee merged commit 51533b8 into googleapis:master Jun 22, 2021
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants