8000 samples: add default leader options samples by zoercai · Pull Request #428 · googleapis/python-spanner · GitHub
[go: up one dir, main page]
Skip to content

samples: add default leader options samples#428

Merged
larkee merged 20 commits intogoogleapis:masterfrom
zoercai:cmmr-1-samples
Aug 9, 2021
Merged

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

Conversation

@zoercai
Copy link
Contributor
@zoercai zoercai commented Jul 26, 2021 •
edited
Loading

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

@zoercai zoercai requested a review from a team July 26, 2021 12:40
@zoercai zoercai requested a review from a team as a code owner July 26, 2021 12:40
@zoercai zoercai requested review from crwilcox and removed request for a team July 26, 2021 12:40
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner API. label Jul 26, 2021
@snippet-bot
Copy link
8000 snippet-bot bot commented Jul 26, 2021 •
edited
Loading

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 product-auto-label bot added the samples Issues that are directly related to samples. label Jul 26, 2021
@zoercai zoercai requested a review from larkee July 26, 2021 12:40
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 26, 2021
@zoercai zoercai requested a review from hengfengli July 26, 2021 12:41
larkee
larkee suggested changes Jul 27, 2021
View reviewed changes
Copy link
Contributor
@larkee larkee left a comment

Choose a reason for hiding this comment

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

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
larkee approved these changes Jul 28, 2021
View reviewed changes
Copy link
Contributor
@larkee larkee left a comment

Choose a reason for hiding this comment

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

LGTM once the implementation is merged and released 👍

@larkee larkee changed the title samples: add default leader options samples 3ce1ed7 samples: add default leader options samples Jul 28, 2021
@larkee 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 zoercai added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 28, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 28, 2021
@zoercai
Merge branch 'master' of github.com:googleapis/python-spanner into cm…
3c88ef0 
…mr-1-samples

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

Choose a reason for hiding this comment

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

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

@larkee 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 zoercai added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 5, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 5, 2021
@hengfengli
Copy link
hengfengli commented Aug 9, 2021

@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 larkee added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 9, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 9, 2021
@larkee larkee merged commit 86293f9 into googleapis:master Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crwilcox crwilcox Awaiting requested review from crwilcox

@hengfengli hengfengli Awaiting requested review from hengfengli

1 more reviewer

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

@zoercai @hengfengli @larkee @yoshi-kokoro
0