feat: add `Database.list_tables` method #219

tswast · 2021-01-29T22:40:12Z

Adds Table-management methods required by PSO data validation tool.

Fixes #156 🦕

larkee

LG, just a couple design details to work out regarding the Table methods 👍

google/cloud/spanner_v1/database.py

google/cloud/spanner_v1/table.py

larkee · 2021-02-01T08:38:53Z

google/cloud/spanner_v1/table.py

+        self.table_id = table_id
+        self._database = database
+
+    def get_schema(self):


I think instead of having get_schema, it would make more sense to match the rest of the library and the BigQuery implementation by having this as a property. What do you think?

That would be more consistent. My main worry with a property is it might be less clear that table.schema makes an API call compared to table.get_schema().

That is a good point. For the other resource objects (i.e. Instance, Backup, Database) we store local copies of properties and have a reload() method to make an RPC call to update them. I think it may make sense to stick to this design, especially if we decide to support create, update, and delete on the Table object.

I would initialize a private schema attribute as None and then call reload in schema if it is None. This means the property would make one API call at most. Then, if the user is expecting the schema to change frequently, they can all reload themselves to ensure the schema remains in sync with production. WDYT?

I would initialize a private schema attribute as None and then call reload in schema if it is None. This means the property would make one API call at most. Then, if the user is expecting the schema to change frequently, they can all reload themselves to ensure the schema remains in sync with production. WDYT?

I think that's a good approach. In BigQuery we didn't have this logic, so customers would surprisingly get None values, which caused all sorts of issues. I'll make this a property and add a reload method.

Updated. Also added a exists() method to match Database.reload()'s NotFound exception behavior.

The NUMERIC column is not included in the emulator system tests.

tswast · 2021-02-01T23:47:56Z

I'm marking this as ready for review, though I suspect I'll need to add some unit tests for coverage.

larkee · 2021-02-02T04:11:20Z

The new Table type will also need documentation similar to database-api and database-usage so that documentation for the type and how to use it can be generated.

tswast · 2021-02-02T15:37:10Z

Docs added:

larkee · 2021-02-08T07:35:23Z

docs/table-usage.rst

+
+.. code:: python
+
+    table = google.cloud.spanner_v1.table.Table(


Please add a table() method on the Database object which creates the Table object to be consistent with the other classes e.g.

client = spanner.Client() instance = client.instance(instance_id) database = instance.database(database_id) table = database.table(table_id)

Done! Thanks for your patience while I was OOO.

Edit: added in the following commit: 1250d8a

tswast · 2021-02-22T21:22:12Z

@larkee I've made the requested changes and synced with latest master branch. Should be ready for another round of review.

tswast · 2021-02-23T22:49:54Z

Test failure

______________________ TestBackupAPI.test_backup_workflow ______________________

self = <tests.system.test_system.TestBackupAPI testMethod=test_backup_workflow>

    def test_backup_workflow(self):
        from datetime import datetime
        from datetime import timedelta
        from pytz import UTC

        instance = Config.INSTANCE
        backup_id = "backup_id" + unique_resource_id("_")
        expire_time = datetime.utcnow() + timedelta(days=3)
        expire_time = expire_time.replace(tzinfo=UTC)

        # Create backup.
        backup = instance.backup(
            backup_id,
            database=self._db,
            expire_time=expire_time,
            version_time=self.database_version_time,
        )
        operation = backup.create()
        self.to_delete.append(backup)

        # Check metadata.
        metadata = operation.metadata
        self.assertEqual(backup.name, metadata.name)
        self.assertEqual(self._db.name, metadata.database)
        operation.result()

        # Check backup object.
        backup.reload()
        self.assertEqual(self._db.name, backup._database)
        self.assertEqual(expire_time, backup.expire_time)
        self.assertIsNotNone(backup.create_time)
        self.assertEqual(self.database_version_time, backup.version_time)
        self.assertIsNotNone(backup.size_bytes)
        self.assertIsNotNone(backup.state)

        # Update with valid argument.
        valid_expire_time = datetime.utcnow() + timedelta(days=7)
        valid_expire_time = valid_expire_time.replace(tzinfo=UTC)
        backup.update_expire_time(valid_expire_time)
        self.assertEqual(valid_expire_time, backup.expire_time)

        # Restore database to same instance.
        restored_id = "restored_db" + unique_resource_id("_")
        database = instance.database(restored_id)
        self.to_drop.append(database)
        operation = database.restore(source=backup)
        restored_db = operation.result()
>       self.assertEqual(
            self.database_version_time, restored_db.restore_info.backup_info.create_time
        )
E       AssertionError: datetime.datetime(2021, 2, 22, 21, 31, 33[16 chars]UTC>) != DatetimeWithNanoseconds(2021, 2, 22, 21, [40 chars].utc)

appears to be a problem with master branch. I'll try merging from upstream again.

larkee

LGTM other than a minor docs fix 👍

docs/table-usage.rst

product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner API. label Jan 29, 2021

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jan 29, 2021

larkee reviewed Feb 1, 2021

View reviewed changes

tswast added 2 commits February 1, 2021 10:53

feat: add Database.list_tables method

3f9cc92

update docs, add tests

40a60e3

tswast force-pushed the issue156-list_tables branch from 134f1c8 to 40a60e3 Compare February 1, 2021 23:43

remove numeric from get_schema test

e016b0f

The NUMERIC column is not included in the emulator system tests.

tswast marked this pull request as ready for review February 1, 2021 23:47

tswast requested a review from a team as a code owner February 1, 2021 23:47

add unit tests

f407289

tswast added 2 commits February 2, 2021 09:30

add docs for table api and usage

21395ec

fix link to Field class

2b566fa

tswast added 2 commits February 2, 2021 09:38

typo in table constructor docs

66aa192

add reload and exists methods

bfbc105

larkee reviewed Feb 8, 2021

View reviewed changes

tswast added 3 commits February 17, 2021 09:00

Merge remote-tracking branch 'upstream/master' into issue156-list_tables

fdd6bf9

feat: add table method to database

1250d8a

update usage docs to use factory method

3f6eb34

tswast requested a review from larkee February 17, 2021 15:28

Merge remote-tracking branch 'upstream/master' into issue156-list_tables

c7cd934

address warning in GitHub UI for sphinx header

611b27f

Merge remote-tracking branch 'upstream/master' into issue156-list_tables

ef978df

tswast mentioned this pull request Feb 23, 2021

ibis_cloud_spanner folder added GoogleCloudPlatform/professional-services-data-validator#186

Merged

larkee approved these changes Feb 26, 2021

View reviewed changes

docs/table-usage.rst Outdated Show resolved Hide resolved

Update docs/table-usage.rst

d91e666

larkee added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 1, 2021

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 1, 2021

larkee merged commit 28bde8c into googleapis:master Mar 1, 2021

tswast deleted the issue156-list_tables branch March 1, 2021 18:06

dependabot bot mentioned this pull request Mar 15, 2021

Bump google-cloud-spanner from 3.1.0 to 3.2.0 shoma/spanner-cli#106

Closed

tswast mentioned this pull request Apr 1, 2021

cleanup spanner connector to delete duplicate code from google-cloud-spanner library GoogleCloudPlatform/professional-services-data-validator#225

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `Database.list_tables` method #219

feat: add `Database.list_tables` method #219

tswast commented Jan 29, 2021 •

edited by larkee

Loading

larkee left a comment

larkee Feb 1, 2021

tswast Feb 1, 2021

larkee Feb 2, 2021

tswast Feb 2, 2021

tswast Feb 2, 2021

tswast commented Feb 1, 2021

larkee commented Feb 2, 2021 •

edited

Loading

tswast commented Feb 2, 2021

larkee Feb 8, 2021

tswast Feb 17, 2021 •

edited

Loading

tswast commented Feb 22, 2021

tswast commented Feb 23, 2021

larkee left a comment


		.. code:: python

		table = google.cloud.spanner_v1.table.Table(

feat: add Database.list_tables method #219

feat: add Database.list_tables method #219

Conversation

tswast commented Jan 29, 2021 • edited by larkee Loading

larkee left a comment

Choose a reason for hiding this comment

larkee Feb 1, 2021

Choose a reason for hiding this comment

tswast Feb 1, 2021

Choose a reason for hiding this comment

larkee Feb 2, 2021

Choose a reason for hiding this comment

tswast Feb 2, 2021

Choose a reason for hiding this comment

tswast Feb 2, 2021

Choose a reason for hiding this comment

tswast commented Feb 1, 2021

larkee commented Feb 2, 2021 • edited Loading

tswast commented Feb 2, 2021

larkee Feb 8, 2021

Choose a reason for hiding this comment

tswast Feb 17, 2021 • edited Loading

Choose a reason for hiding this comment

tswast commented Feb 22, 2021

tswast commented Feb 23, 2021

larkee left a comment

Choose a reason for hiding this comment

feat: add `Database.list_tables` method #219

feat: add `Database.list_tables` method #219

tswast commented Jan 29, 2021 •

edited by larkee

Loading

larkee commented Feb 2, 2021 •

edited

Loading

tswast Feb 17, 2021 •

edited

Loading