From 42883ac58c9f0ac918e50b516522ff59756a25ab Mon Sep 17 00:00:00 2001 From: Zoe Cai Date: Wed, 20 Jan 2021 13:25:24 +1100 Subject: [PATCH 1/5] Backup changes --- google/cloud/spanner_v1/backup.py | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/google/cloud/spanner_v1/backup.py b/google/cloud/spanner_v1/backup.py index 405a9e2be2..2277a33fce 100644 --- a/google/cloud/spanner_v1/backup.py +++ b/google/cloud/spanner_v1/backup.py @@ -51,14 +51,23 @@ class Backup(object): :param expire_time: (Optional) The expire time that will be used to create the backup. Required if the create method needs to be called. + + :type version_time: :class:`datetime.datetime` + :param version_time: (Optional) The version time that was specified for + the externally consistent copy of the database. If + not present, it is the same as the `create_time` of + the backup. """ - def __init__(self, backup_id, instance, database="", expire_time=None): + def __init__( + self, backup_id, instance, database="", expire_time=None, version_time=None + ): self.backup_id = backup_id self._instance = instance self._database = database self._expire_time = expire_time self._create_time = None + self._version_time = version_time self._size_bytes = None self._state = None self._referencing_databases = None @@ -109,6 +118,16 @@ def create_time(self): """ return self._create_time + @property + def version_time(self): + """Version time of this backup. + + :rtype: :class:`datetime.datetime` + :returns: a datetime object representing the version time of + this backup + """ + return self._version_time + @property def size_bytes(self): """Size of this backup in bytes. @@ -190,7 +209,11 @@ def create(self): raise ValueError("database not set") api = self._instance._client.database_admin_api metadata = _metadata_with_prefix(self.name) - backup = BackupPB(database=self._database, expire_time=self.expire_time,) + backup = BackupPB( + database=self._database, + expire_time=self.expire_time, + version_time=self.version_time, + ) future = api.create_backup( parent=self._instance.name, @@ -228,6 +251,7 @@ def reload(self): self._database = pb.database self._expire_time = pb.expire_time self._create_time = pb.create_time + self._version_time = pb.version_time self._size_bytes = pb.size_bytes self._state = BackupPB.State(pb.state) self._referencing_databases = pb.referencing_databases From 9ce3b308300c6a358298e479a8c99b277afc0c4a Mon Sep 17 00:00:00 2001 From: Zoe Cai Date: Wed, 20 Jan 2021 13:25:38 +1100 Subject: [PATCH 2/5] Basic tests --- tests/system/test_system.py | 24 ++++++++++++++++++++++-- tests/unit/test_backup.py | 17 ++++++++++++++--- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 71ebdb3d7c..24c229a52d 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -672,9 +672,16 @@ def test_backup_workflow(self): backup_id = "backup_id" + unique_resource_id("_") expire_time = datetime.utcnow() + timedelta(days=3) expire_time = expire_time.replace(tzinfo=UTC) + version_time = datetime.utcnow() - timedelta(days=5) + version_time = version_time.replace(tzinfo=UTC) # Create backup. - backup = instance.backup(backup_id, database=self._db, expire_time=expire_time) + backup = instance.backup( + backup_id, + database=self._db, + expire_time=expire_time, + version_time=version_time, + ) operation = backup.create() self.to_delete.append(backup) @@ -689,6 +696,7 @@ def test_backup_workflow(self): self.assertEqual(self._db.name, backup._database) self.assertEqual(expire_time, backup.expire_time) self.assertIsNotNone(backup.create_time) + self.assertEqual(version_time, backup.version_time) self.assertIsNotNone(backup.size_bytes) self.assertIsNotNone(backup.state) @@ -805,9 +813,14 @@ def test_list_backups(self): instance = Config.INSTANCE expire_time_1 = datetime.utcnow() + timedelta(days=21) expire_time_1 = expire_time_1.replace(tzinfo=UTC) + version_time_1 = datetime.utcnow() - timedelta(days=5) + version_time_1 = version_time_1(tzinfo=UTC) backup1 = Config.INSTANCE.backup( - backup_id_1, database=self._dbs[0], expire_time=expire_time_1 + backup_id_1, + database=self._dbs[0], + expire_time=expire_time_1, + version_time=version_time_1, ) expire_time_2 = datetime.utcnow() + timedelta(days=1) @@ -847,6 +860,13 @@ def test_list_backups(self): for backup in instance.list_backups(filter_=filter_): self.assertEqual(backup.name, backup2.name) + # List backups filtered by version time. + filter_ = 'version_time > "{0}"'.format( + create_time_compare.strftime("%Y-%m-%dT%H:%M:%S.%fZ") + ) + for backup in instance.list_backups(filter_=filter_): + self.assertEqual(backup.name, backup2.name) + # List backups filtered by expire time. filter_ = 'expire_time > "{0}"'.format( expire_time_1.strftime("%Y-%m-%dT%H:%M:%S.%fZ") diff --git a/tests/unit/test_backup.py b/tests/unit/test_backup.py index 748c460291..063dc5db0d 100644 --- a/tests/unit/test_backup.py +++ b/tests/unit/test_backup.py @@ -273,12 +273,21 @@ def test_create_success(self): api.create_backup.return_value = op_future instance = _Instance(self.INSTANCE_NAME, client=client) - timestamp = self._make_timestamp() + version_timestamp = self._make_timestamp() + expire_timestamp = self._make_timestamp() backup = self._make_one( - self.BACKUP_ID, instance, database=self.DATABASE_NAME, expire_time=timestamp + self.BACKUP_ID, + instance, + database=self.DATABASE_NAME, + expire_time=expire_timestamp, + version_time=version_timestamp, ) - backup_pb = Backup(database=self.DATABASE_NAME, expire_time=timestamp,) + backup_pb = Backup( + database=self.DATABASE_NAME, + expire_time=expire_timestamp, + version_time=version_timestamp, + ) future = backup.create() self.assertIs(future, op_future) @@ -437,6 +446,7 @@ def test_reload_success(self): name=self.BACKUP_NAME, database=self.DATABASE_NAME, expire_time=timestamp, + version_time=timestamp, create_time=timestamp, size_bytes=10, state=1, @@ -452,6 +462,7 @@ def test_reload_success(self): self.assertEqual(backup.database, self.DATABASE_NAME) self.assertEqual(backup.expire_time, timestamp) self.assertEqual(backup.create_time, timestamp) + self.assertEqual(backup.version_time, timestamp) self.assertEqual(backup.size_bytes, 10) self.assertEqual(backup.state, Backup.State.CREATING) self.assertEqual(backup.referencing_databases, []) From 5464403ebff0514d7502a716babc5a85cff66617 Mon Sep 17 00:00:00 2001 From: Zoe Cai Date: Wed, 20 Jan 2021 18:50:19 +1100 Subject: [PATCH 3/5] Add system tests --- tests/system/test_system.py | 74 +++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 24c229a52d..e0598ee15d 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -717,6 +717,80 @@ def test_backup_workflow(self): backup.delete() self.assertFalse(backup.exists()) + def test_backup_version_time_defaults_to_create_time(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,) + operation = backup.create() + self.to_delete.append(backup) + + # Check backup object. + backup.reload() + self.assertEqual(self._db.name, backup._database) + self.assertIsNotNone(backup.create_time) + self.assertEqual(backup.create_time, backup.version_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) + operation.result() + + database.drop() + backup.delete() + self.assertFalse(backup.exists()) + + def test_create_backup_invalid_version_time_past(self): + from datetime import datetime + from pytz import UTC + + backup_id = "backup_id" + unique_resource_id("_") + expire_time = datetime.utcnow() + timedelta(days=3) + expire_time = expire_time.replace(tzinfo=UTC) + version_time = datetime.utcnow() - timedelta(days=10) + version_time = version_time.replace(tzinfo=UTC) + + backup = Config.INSTANCE.backup( + backup_id, + database=self._db, + expire_time=expire_time, + version_time=version_time, + ) + + with self.assertRaises(exceptions.InvalidArgument): + op = backup.create() + op.result() + + def test_create_backup_invalid_version_time_future(self): + from datetime import datetime + from pytz import UTC + + backup_id = "backup_id" + unique_resource_id("_") + expire_time = datetime.utcnow() + timedelta(days=3) + expire_time = expire_time.replace(tzinfo=UTC) + version_time = datetime.utcnow() + timedelta(days=2) + version_time = version_time.replace(tzinfo=UTC) + + backup = Config.INSTANCE.backup( + backup_id, + database=self._db, + expire_time=expire_time, + version_time=version_time, + ) + + with self.assertRaises(exceptions.InvalidArgument): + op = backup.create() + op.result() + def test_restore_to_diff_instance(self): from datetime import datetime from datetime import timedelta From 6477f830917281285845df63b6fab077b661a56f Mon Sep 17 00:00:00 2001 From: Zoe Cai Date: Wed, 20 Jan 2021 21:03:11 +1100 Subject: [PATCH 4/5] Fix system tests --- google/cloud/spanner_v1/instance.py | 22 +++++++++++++++++++--- tests/system/test_system.py | 14 +++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/google/cloud/spanner_v1/instance.py b/google/cloud/spanner_v1/instance.py index b422c57afd..ffaed41c91 100644 --- a/google/cloud/spanner_v1/instance.py +++ b/google/cloud/spanner_v1/instance.py @@ -400,7 +400,7 @@ def list_databases(self, page_size=None): ) return page_iter - def backup(self, backup_id, database="", expire_time=None): + def backup(self, backup_id, database="", expire_time=None, version_time=None): """Factory to create a backup within this instance. :type backup_id: str @@ -415,13 +415,29 @@ def backup(self, backup_id, database="", expire_time=None): :param expire_time: Optional. The expire time that will be used when creating the backup. Required if the create method needs to be called. + + :type version_time: :class:`datetime.datetime` + :param version_time: + Optional. The version time that will be used to create the externally + consistent copy of the database. If not present, it is the same as + the `create_time` of the backup. """ try: return Backup( - backup_id, self, database=database.name, expire_time=expire_time + backup_id, + self, + database=database.name, + expire_time=expire_time, + version_time=version_time, ) except AttributeError: - return Backup(backup_id, self, database=database, expire_time=expire_time) + return Backup( + backup_id, + self, + database=database, + expire_time=expire_time, + version_time=version_time, + ) def list_backups(self, filter_="", page_size=None): """List backups for the instance. diff --git a/tests/system/test_system.py b/tests/system/test_system.py index e0598ee15d..30c4d4a1b6 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -672,7 +672,7 @@ def test_backup_workflow(self): backup_id = "backup_id" + unique_resource_id("_") expire_time = datetime.utcnow() + timedelta(days=3) expire_time = expire_time.replace(tzinfo=UTC) - version_time = datetime.utcnow() - timedelta(days=5) + version_time = datetime.utcnow() - timedelta(minutes=5) version_time = version_time.replace(tzinfo=UTC) # Create backup. @@ -732,6 +732,12 @@ def test_backup_version_time_defaults_to_create_time(self): 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) @@ -751,6 +757,7 @@ def test_backup_version_time_defaults_to_create_time(self): def test_create_backup_invalid_version_time_past(self): from datetime import datetime + from datetime import timedelta from pytz import UTC backup_id = "backup_id" + unique_resource_id("_") @@ -772,6 +779,7 @@ def test_create_backup_invalid_version_time_past(self): def test_create_backup_invalid_version_time_future(self): from datetime import datetime + from datetime import timedelta from pytz import UTC backup_id = "backup_id" + unique_resource_id("_") @@ -887,8 +895,8 @@ def test_list_backups(self): instance = Config.INSTANCE expire_time_1 = datetime.utcnow() + timedelta(days=21) expire_time_1 = expire_time_1.replace(tzinfo=UTC) - version_time_1 = datetime.utcnow() - timedelta(days=5) - version_time_1 = version_time_1(tzinfo=UTC) + version_time_1 = datetime.utcnow() - timedelta(minutes=5) + version_time_1 = version_time_1.replace(tzinfo=UTC) backup1 = Config.INSTANCE.backup( backup_id_1, From 40da42c045ef72a65174b8cd9bcfab873b52c50f Mon Sep 17 00:00:00 2001 From: Zoe Cai Date: Thu, 21 Jan 2021 15:28:01 +1100 Subject: [PATCH 5/5] Add retention period to backup systests --- tests/system/test_system.py | 26 +++++++++++++++++--------- tests/unit/test_backup.py | 6 +++++- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 30c4d4a1b6..65ef27425a 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -600,6 +600,22 @@ def setUpClass(cls): op1.result(30) # raises on failure / timeout. op2.result(30) # raises on failure / timeout. + # Add retention period for backups + retention_period = "7d" + ddl_statements = DDL_STATEMENTS + [ + "ALTER DATABASE {}" + " SET OPTIONS (version_retention_period = '{}')".format( + cls.DATABASE_NAME, retention_period + ) + ] + db = Config.INSTANCE.database( + cls.DATABASE_NAME, pool=pool, ddl_statements=ddl_statements + ) + operation = db.update_ddl(ddl_statements) + # We want to make sure the operation completes. + operation.result(240) # raises on failure / timeout. + db.reload() + current_config = Config.INSTANCE.configuration_name same_config_instance_id = "same-config" + unique_resource_id("-") create_time = str(int(time.time())) @@ -672,7 +688,7 @@ def test_backup_workflow(self): backup_id = "backup_id" + unique_resource_id("_") expire_time = datetime.utcnow() + timedelta(days=3) expire_time = expire_time.replace(tzinfo=UTC) - version_time = datetime.utcnow() - timedelta(minutes=5) + version_time = datetime.utcnow() - timedelta(seconds=5) version_time = version_time.replace(tzinfo=UTC) # Create backup. @@ -744,14 +760,6 @@ def test_backup_version_time_defaults_to_create_time(self): self.assertIsNotNone(backup.create_time) self.assertEqual(backup.create_time, backup.version_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) - operation.result() - - database.drop() backup.delete() self.assertFalse(backup.exists()) diff --git a/tests/unit/test_backup.py b/tests/unit/test_backup.py index 063dc5db0d..bf6ce68a84 100644 --- a/tests/unit/test_backup.py +++ b/tests/unit/test_backup.py @@ -266,6 +266,9 @@ def test_create_database_not_set(self): def test_create_success(self): from google.cloud.spanner_admin_database_v1 import Backup + from datetime import datetime + from datetime import timedelta + from pytz import UTC op_future = object() client = _Client() @@ -273,7 +276,8 @@ def test_create_success(self): api.create_backup.return_value = op_future instance = _Instance(self.INSTANCE_NAME, client=client) - version_timestamp = self._make_timestamp() + version_timestamp = datetime.utcnow() - timedelta(minutes=5) + version_timestamp = version_timestamp.replace(tzinfo=UTC) expire_timestamp = self._make_timestamp() backup = self._make_one( self.BACKUP_ID,