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

docs(samples): add samples for CMMR phase 2 #672

Merged
rahul2393 merged 4 commits intogoogleapis:mainfrom
rahul2393:spanner-custom-replication-topology
Oct 3, 2022
Merged

docs(samples): add samples for CMMR phase 2 #672
rahul2393 merged 4 commits intogoogleapis:mainfrom
rahul2393:spanner-custom-replication-topology

Conversation

@rahul2393
Copy link
Copy Markdown
Contributor

@rahul2393 rahul2393 commented Jan 25, 2022

Tests and samples for user-managed instance configurations

BEGIN_COMMIT_OVERRIDE
docs(samples): add samples for CMMR phase 2
END_COMMIT_OVERRIDE

@rahul2393 rahul2393 requested a review from a team January 25, 2022 06:36
@rahul2393 rahul2393 requested review from a team as code owners January 25, 2022 06:36
@snippet-bot
Copy link
Copy Markdown

snippet-bot bot commented Jan 25, 2022

Here is the summary of changes.

You are about to add 4 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 api: spanner Issues related to the googleapis/python-spanner API. label Jan 25, 2022
@rahul2393 rahul2393 changed the title [DO NOT MERGE] support customer managed instance configurations feat: [DO NOT MERGE] support customer managed instance configurations Jan 25, 2022
@vi3k6i5 vi3k6i5 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 25, 2022
Copy link
Copy Markdown
Contributor

@olavloite olavloite left a comment

Choose a reason for hiding this comment

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

LGTM

I did not add an explicit approval as it's not ready for merge yet.

base_config = spanner_client.instance_admin_api.get_instance_config(
name="{}/instanceConfigs/{}".format(spanner_client.project_name, base_instance_config))
replicas = []
for replica in base_config.replicas:
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.

nit: Add a comment that it is required that the custom configuration contains all the replicas of the base config + at least one optional replica. That makes it clearer to the user why we are doing this.

def test_update_instance_config(capsys, user_managed_instance_config_id):
snippets.update_instance_config(user_managed_instance_config_id)
out, _ = capsys.readouterr()
assert "custom-updated-sample-config" in out
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.

nit: Add something like 'Updated ...' to the assert here, so it's easier to see that this really verifies that the function succeeded.

def test_create_instance_config(capsys, user_managed_instance_config_id):
snippets.create_instance_config("base-config", user_managed_instance_config_id)
out, _ = capsys.readouterr()
assert "custom-sample-config" in out
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.

nit: Add something like 'Created ...' to the assert here, so it's easier to see that this really verifies that the function succeeded.

@rahul2393 rahul2393 force-pushed the spanner-custom-replication-topology branch from a03c43b to 7f7af44 Compare September 28, 2022 14:11
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Sep 28, 2022
@rahul2393 rahul2393 changed the title feat: [DO NOT MERGE] support customer managed instance configurations feat: add samples for CMMR phase 2 Sep 28, 2022
@rahul2393 rahul2393 force-pushed the spanner-custom-replication-topology branch from c0d1b48 to b116734 Compare September 28, 2022 14:40
@rahul2393 rahul2393 force-pushed the spanner-custom-replication-topology branch from b116734 to 6f4f268 Compare September 28, 2022 15:02
@asthamohta asthamohta removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 28, 2022
@rahul2393 rahul2393 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 29, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 29, 2022
@rahul2393 rahul2393 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 1, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 1, 2022
@rahul2393 rahul2393 merged commit 4282340 into googleapis:main Oct 3, 2022
@parthea parthea changed the title feat: add samples for CMMR phase 2 docs(samples): add samples for CMMR phase 2 Oct 4, 2022
@parthea parthea added the release-please:force-run To run release-please label Oct 4, 2022
@release-please release-please bot removed the release-please:force-run To run release-please label Oct 4, 2022
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. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants