chore(tests): system tests for autogen API#1151
chore(tests): system tests for autogen API#1151gkevinzheng merged 6 commits intoautogen-feature-branchfrom
Conversation
daniel-sanche
left a comment
There was a problem hiding this comment.
a couple small comments, but looks good!
| # Instantiate a meaningless client to get project name | ||
| # from environment variables. | ||
| client = ClientWithProject() | ||
| yield client.project |
There was a problem hiding this comment.
Creating a client just for this seems a bit messy.
Looking through the ClientWithProject code, it seems like it just uses google.auth.default() for this
I think you should also check os.getenv("GOOGLE_CLOUD_PROJECT") first though, so we can configure a different project if needed (And maybe put this up with the constants as ADMIN_TEST_PROJECT or something so it's discoverable?)
|
|
||
|
|
||
| @CrossSync.convert | ||
| @CrossSync.pytest_fixture(scope="session") |
There was a problem hiding this comment.
This is making me realize @CrossSync.pytest_fixture should always do @CrossSync.convert automatically. We should change that later
| yield instances | ||
|
|
||
| for instance in instances: | ||
| await instance_admin_client.delete_instance(name=instance.name) |
There was a problem hiding this comment.
IIRC I think you might need a try/finally block here (and delete backups), to make sure these are always cleaned up even if there's an exception
async def instances_to_delete(instance_admin_client):
instances = []
try:
yield instances
finally:
for instance in instances:
await instance_admin_client.delete_instance(name=instance.name)
| @CrossSync.convert | ||
| async def create_backup( | ||
| instance_admin_client, table_admin_client, instance, table, backups_to_delete | ||
| ) -> admin_v2.Backup: |
There was a problem hiding this comment.
docstring? It would be good to know why this helper is needed
| ) | ||
|
|
||
|
|
||
| @CrossSync.convert |
There was a problem hiding this comment.
nit: I think @CrossSync.pytest already does convert automatically, so this should be redundant. but you can double-check
| client = ClientWithProject() | ||
| yield client.project | ||
| _, default_project = google.auth.default() | ||
| yield os.getenv("ADMIN_TEST_PROJECT") or default_project |
There was a problem hiding this comment.
You should check for GOOGLE_CLOUD_PROJECT, since that's the standard (and is used in the data client)
If you want, you can check for both, but if AMIN_TEST_PROJECT isn't set, it should fall back to the standard one
…ration (#1177) * chore: Removed old admin_v2 GAPIC layer (#1111) * feat!: Generated Selective GAPIC layer for Admin API (#1112) * chore: Updated service YAML by making all methods in BigtableInstanceAdmin public (#1113) * refactor: Refactored classic client to use new Admin API (#1114) * refactor: Refactored classic client to use new Admin API * added newline after gapic_version files * fix: Made generate_consistency_token and check_consistency public (#1116) methods * feat: Consistency polling + restore table for sync client in admin (#1117) * feat: Prototyped handwritten layer * Added newlines * linting * Added docstrings for restore table and consistency token polling; removed gc_rule * docs: owlbot related changes (#1133) * docs: owlbot related changes * Addressed PR feedback + made changes to toc.yml for docs pipeline * Fixed type hint * linting + added validation for admin section * linting + added noqas to owlbot lines * tests: Tests for sync client + fixes + client library versioning (#1132) * tests: Tests for sync client + fixes + client library versioning * Removed raise exception * linting + name changes in tests + added test for timeout * linting * Fixed tests on Python 3.7 * feat: Proto-plus modifications for enforcing strict oneofs (#1126) * feat: Proto-plus modifications for enforcing strict oneofs * Added template directory + changed unit tests to pytest * Finished README * linting * Added source of truth comment * feat: Reworked the wait_for_consistency call (#1144) * feat: Reworked the wait_for_consistency call * linting * Update google/cloud/bigtable/admin_v2/overlay/services/bigtable_table_admin/client.py Co-authored-by: Mattie Fu <[email protected]> * Improved documentation * linting again * linting --------- Co-authored-by: Mattie Fu <[email protected]> * feat: Async consistency polling harness (#1142) * feat: Async consistency polling harness * Fixed AsyncMock issue in Python 3.7 * Reworked async_consistency and added async client to __init__.py * linting * addressed review feedback * linting * feat: Restore Table LRO rework + async restore table (#1148) * chore(tests): system tests for autogen API (#1151) * tests: system tests for autogen API * Fixed async system tests * addressed review feedback * Fixed system test failure at the end of a test run * Linting * more linting * chore: Moved Admin API from google.cloud.bigtable.admin_v2 back to google.cloud.bigtable_admin_v2 (#1153) * chore: Removed autogenerated files from the feature branch (#1170) * chore: Merged selective GAPIC autogenerated changes into feature branch (#1175) * chore: Merged selective GAPIC owlbot changes into feature branch * linting * changed comment text * Removed redundant items * Fixed owlbot infinitely appending text * Added comments + fixed indentation in Owlbot * Added anonymous credentials to client tests * Fixed project ID issue in system tests * Fixed docstrings and skipped system tests on emulator. --------- Co-authored-by: Mattie Fu <[email protected]>
Added system tests for the autogenerated Admin API. While working on the system tests, a number of issues popped up:
WaitForConsistencyRequestandAsyncRestoreTableOperationwere not aliased under thegoogle.cloud.bigtable.admin_v2package, so they were added to the appropriate__init__.pyfiles.wait_for_consistencypolling harness has been extended to an indefinite timeout.google-api-coreto facilitate thatTest results: