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.

Sign up for GitHub

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

Jump to bottom

docs: samples and tests for admin database APIs #1099

Merged
merged 13 commits into from
Feb 26, 2024
Merged

docs: samples and tests for admin database APIs #1099

merged 13 commits into from
Feb 26, 2024

Conversation

rahul2393
Copy link
Contributor

Notes

  • Adds samples and tests for auto-generated database admin APIs.
  • The sample code style is mostly consistent with our existing samples.

depends on: #1065

@rahul2393 rahul2393 requested review from a team as code owners February 9, 2024 09:13
@product-auto-label product-auto-label bot added the size: xl Pull request size is extra large. label Feb 9, 2024
@snippet-bot Snippet Bot
Copy link

snippet-bot bot commented Feb 9, 2024 •
edited
Loading

Here is the summary of changes.

You are about to add 29 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 api: spanner Issues related to the googleapis/python-spanner API. samples Issues that are directly related to samples. labels Feb 9, 2024
@rahul2393 rahul2393 force-pushed the database_admin_samples branch 2 times, most recently from c1676a7 to 724b7e8 Compare February 19, 2024 16:58
@rahul2393 rahul2393 force-pushed the database_admin_samples branch from 724b7e8 to 99367f2 Compare February 20, 2024 03:45
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: xl Pull request size is extra large. labels Feb 20, 2024
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Feb 21, 2024
@harshachinta
Copy link
Contributor

Reviewed samples.py. Will look at pg samples tomorrow.

harshachinta
harshachinta reviewed Feb 22, 2024
View reviewed changes
samples/samples/admin/pg_samples.py
OPERATION_TIMEOUT_SECONDS = 240


def create_table_with_datatypes(instance_id, database_id):
Copy link
Contributor

Choose a reason for hiding this comment

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

The existing pg_snippets already has this sample with autogenerated code. Do we still need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we need this because there is dependency on this sample in other tests.

harshachinta
harshachinta reviewed Feb 22, 2024
View reviewed changes
samples/samples/admin/pg_samples_test.py Outdated
assert "Created Venues table on database" in out


@pytest.mark.dependency(name="add_jsonb_column", depends=["insert_datatypes_data"])
Copy link
Contributor

Choose a reason for hiding this comment

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

The dependency for this test insert_datatypes_data is not there. How do we validate the sample since it will always get skipped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated dependency to create_table_with_datatypes since we need tables to be created.

harshachinta
harshachinta reviewed Feb 22, 2024
View reviewed changes
samples/samples/admin/pg_samples.py
# limitations under the License.

"""This application demonstrates how to do basic operations using Cloud
Spanner PostgreSql dialect.
Copy link
Contributor

Choose a reason for hiding this comment

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

Will we be adding create_database sample for postgres? I think that is needed for customers to understand how to set a PG database.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@rahul2393 rahul2393 force-pushed the database_admin_samples branch from 81530ac to f18d0e1 Compare February 23, 2024 05:10
@rahul2393 rahul2393 force-pushed the database_admin_samples branch from d9be1fb to ed20cd5 Compare February 23, 2024 14:34
harshachinta
harshachinta reviewed Feb 24, 2024
View reviewed changes
samples/samples/admin/pg_samples.py
database_dialect=DatabaseDialect.POSTGRESQL,
)

operation = spanner_client.database_admin_api.create_database(request=request)
Copy link
Contributor

Choose a reason for hiding this comment

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

@rahul2393
Do we need to take care of client closure as discussed in Nodejs?
googleapis/nodejs-spanner#1994 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are only initializing admin clients once per client object lifecycle
https://github.com/googleapis/python-spanner/blob/main/google/cloud/spanner_v1/client.py#L244

There is exit method in all of the autogenerated classes which get's called when object is ready for garbage collection.

@rahul2393 rahul2393 merged commit c25376c into main Feb 26, 2024
14 of 16 checks passed
@rahul2393 rahul2393 deleted the database_admin_samples branch February 26, 2024 06:56
@release-please release-please bot mentioned this pull request Feb 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harshachinta harshachinta harshachinta approved these changes

@kweinmeister kweinmeister Awaiting requested review from kweinmeister kweinmeister is a code owner automatically assigned from googleapis/python-samples-reviewers

Assignees
No one assigned
Labels
api: spanner Issues related to the googleapis/python-spanner API. samples Issues that are directly related to samples. size: xl Pull request size is extra large.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rahul2393 @harshachinta