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

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

Merged

Conversation

rahul2393
Copy link
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 review from a team as code owners January 25, 2022 06:36
@snippet-bot
Copy link

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
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
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
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
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 join this conversation on GitHub. Already have an account? Sign in to comment
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