FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

samples: add default leader options samples by zoercai · Pull Request #428 · googleapis/python-spanner · GitHub

This repository was archived by the owner on Jun 8, 2026. It is now read-only.
/ python-spanner Public archive

samples: add default leader options samples - #428

Merged
larkee merged 20 commits into
googleapis:masterfrom
zoercai:cmmr-1-samples
Aug 9, 2021
ConversationCommits (20)ChecksFiles changed
Merged

samples: add default leader options samples#428
larkee merged 20 commits into
googleapis:masterfrom
zoercai:cmmr-1-samples

Conversation

zoercai commented Jul 26, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Samples for #399. Implementation code will be gone once #399 is merged.

zoercai requested a review from a team July 26, 2021 12:40
zoercai requested a review from a team as a code owner July 26, 2021 12:40
zoercai requested review from crwilcox and removed request for a team July 26, 2021 12:40
product-auto-label Bot added the api: spanner Issues related to the googleapis/python-spanner API. label Jul 26, 2021

snippet-bot Bot commented Jul 26, 2021 •
edited
Loading

Copy link
Copy Markdown

Here is the summary of changes.

You are about to add 7 region tags.

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

product-auto-label Bot added the samples Issues that are directly related to samples. label Jul 26, 2021
zoercai requested a review from larkee July 26, 2021 12:40
google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 26, 2021
zoercai requested a review from hengfengli July 26, 2021 12:41
larkee suggested changes Jul 27, 2021
View reviewed changes

larkee 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Currently, you are creating an instance within the spanner_create_database_with_default_leader and spanner_update_database_with_default_leader samples.

This is an issue as if there is a failure later on in the sample, the instance does not get cleaned up correctly. Additionally the second sample fails as it tried to create the same instance.

The instance creation should be removed from the samples and you should instead be using sample_instance, or a pytest fixture that uses sample_instance. This will ensure that the instance clean up is handled correctly.

larkee approved these changes Jul 28, 2021
View reviewed changes

larkee 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM once the implementation is merged and released 👍

larkee changed the title samples: add default leader options samples 3ce1ed7 samples: add default leader options samples Jul 28, 2021
larkee added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 28, 2021
zoercai added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 28, 2021
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 28, 2021
Merge branch 'master' of github.com:googleapis/python-spanner into cm…
3c88ef0 
…mr-1-samples

# Conflicts:
#	tests/system/test_system.py
zoercai added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 29, 2021
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 29, 2021
zoercai added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 29, 2021
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 29, 2021
larkee reviewed Jul 30, 2021
View reviewed changes

larkee 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Having just written the samples for PHP I noticed some inconsistencies. Just a few changes for the list_* samples

Comment thread samples/samples/snippets.py Outdated
Comment thread samples/samples/snippets.py Outdated
larkee removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 3, 2021
zoercai added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 5, 2021
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 5, 2021

hengfengli commented Aug 9, 2021

Copy link
Copy Markdown

@zoercai Why do we get an error Instance already exist?

value = None
from_value = <_InactiveRpcError of RPC that terminated with:
	status = StatusCode.ALREADY_EXISTS
	details = "Instance already exist...message":"Instance already exists: projects/precise-truck-742/instances/google-cloud-1628122209464","grpc_status":6}"
>

>   ???
E   google.api_core.exceptions.AlreadyExists: 409 Instance already exists: projects/precise-truck-742/instances/google-cloud-1628122209464

larkee added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 9, 2021
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 9, 2021
larkee merged commit 86293f9 into googleapis:master Aug 9, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

crwilcox Awaiting requested review from crwilcox
hengfengli Awaiting requested review from hengfengli
1 more reviewer
larkee larkee approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: spanner Issues related to the googleapis/python-spanner API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL