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(spanner): add samples for instance partitions #1168

Merged

Conversation

varuncnaik
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

@varuncnaik varuncnaik requested review from a team as code owners July 24, 2024 04:18
@varuncnaik varuncnaik requested a review from msampathkumar July 24, 2024 04:18
Copy link

snippet-bot bot commented Jul 24, 2024

Here is the summary of changes.

You are about to add 1 region tag.

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 size: m Pull request size is medium. label Jul 24, 2024
@product-auto-label product-auto-label bot added api: spanner Issues related to the googleapis/python-spanner API. samples Issues that are directly related to samples. labels Jul 24, 2024
@@ -188,6 +195,33 @@ def test_create_instance_with_autoscaling_config(capsys, lci_instance_id):
retry_429(instance.delete)()


def test_create_instance_partition(capsys, instance_partition_instance_id):
spanner_client = spanner.Client()
operation = spanner_client.instance_admin_api.create_instance(
Copy link
Contributor

@harshachinta harshachinta Jul 29, 2024

Choose a reason for hiding this comment

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

Can we follow one of the 2 approaches,

  1. We can re-use existing instance that is already created in
    def test_create_instance_explicit(spanner_client, create_instance_id):
    .

This will prevent instance creation time when running tests and also reduce the no of instances that get created.
This test can be marked as dependent to the instance creation test like

@pytest.mark.dependency(name="create_instance_config")
.

  1. If above one is complicated, then can we directly use snippets.create_instance to create an instance instead of rewriting the create logic here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I implemented Approach 2. The challenge with reusing an existing instance is that there are several restrictions around instance partitions. Notably:

  • An instance partition cannot have the same instance config as the parent instance
  • Several Spanner features do not work in instances where the user has created one or more instance partitions.

@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: m Pull request size is medium. labels Jul 30, 2024
@harshachinta harshachinta enabled auto-merge (squash) August 5, 2024 04:59
@harshachinta harshachinta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 5, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 5, 2024
@harshachinta harshachinta added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 6, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 6, 2024
@harshachinta harshachinta merged commit 55f83dc into googleapis:main Aug 6, 2024
11 of 13 checks passed
@varuncnaik varuncnaik deleted the spanner-create-instance-partition branch August 7, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/python-spanner API. samples Issues that are directly related to samples. size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants