feat(default_retention_period): Changes to backup.create API to support null expire time

With the default retention period feature, null value for expiration time
is a valid value. If the expiration time is not specified (null) at the
time of create backup then the backup will be retained for the duration
of maximum retention period.

In this commit, the precondition check on expiration time in the create
backup API path is removed. The condition remains intact for copy backup
API and that would be addressed in a follow-up commit.

Author: satyaprakashg

google/cloud/spanner_v1/backup.py

@@ -53,8 +53,9 @@ class Backup(object):
 
     :type expire_time: :class:`datetime.datetime`
     :param expire_time: (Optional) The expire time that will be used to
-                        create the backup. Required if the create method
-                        needs to be called.
+                        create the backup. If the expire time is not specified
+                        then the backup will be retained for the duration of
+                        maximum retention period.
 
     :type version_time: :class:`datetime.datetime`
     :param version_time: (Optional) The version time that was specified for
@@ -271,8 +272,6 @@ def create(self):
         :raises BadRequest: if the database or expire_time values are invalid
                             or expire_time is not set
         """
-        if not self._expire_time:
-            raise ValueError("expire_time not set")
 
         if not self._database and not self._source_backup:
             raise ValueError("database and source backup both not set")
@@ -295,6 +294,9 @@ def create(self):
         metadata = _metadata_with_prefix(self.name)
 
         if self._source_backup:
+            if not self._expire_time:
+                raise ValueError("expire_time not set")
+
             request = CopyBackupRequest(
                 parent=self._instance.name,
                 backup_id=self.backup_id,

google/cloud/spanner_v1/instance.py

@@ -532,7 +532,8 @@ def backup(
         :type expire_time: :class:`datetime.datetime`
         :param expire_time:
             Optional. The expire time that will be used when creating the backup.
-            Required if the create method needs to be called.
+            If the expire time is not specified then the backup will be retained
+            for the duration of maximum retention period.
 
         :type version_time: :class:`datetime.datetime`
         :param version_time:

tests/unit/test_backup.py

@@ -27,6 +27,8 @@ class _BaseTest(unittest.TestCase):
     DATABASE_NAME = INSTANCE_NAME + "/databases/" + DATABASE_ID
     BACKUP_ID = "backup_id"
     BACKUP_NAME = INSTANCE_NAME + "/backups/" + BACKUP_ID
+    DST_BACKUP_ID = "dst_backup_id"
+    DST_BACKUP_NAME = INSTANCE_NAME + "/backups" + DST_BACKUP_ID
 
     def _make_one(self, *args, **kwargs):
         return self._get_target_class()(*args, **kwargs)
@@ -329,11 +331,45 @@ def test_create_instance_not_found(self):
         )
 
     def test_create_expire_time_not_set(self):
-        instance = _Instance(self.INSTANCE_NAME)
-        backup = self._make_one(self.BACKUP_ID, instance, database=self.DATABASE_NAME)
+        from google.cloud.spanner_admin_database_v1 import Backup
+        from google.cloud.spanner_admin_database_v1 import CreateBackupRequest
+        from datetime import datetime
+        from datetime import timedelta
+        from datetime import timezone
 
-        with self.assertRaises(ValueError):
-            backup.create()
+        op_future = object()
+        client = _Client()
+        api = client.database_admin_api = self._make_database_admin_api()
+        api.create_backup.return_value = op_future
+
+        instance = _Instance(self.INSTANCE_NAME, client=client)
+        version_timestamp = datetime.utcnow() - timedelta(minutes=5)
+        version_timestamp = version_timestamp.replace(tzinfo=timezone.utc)
+        backup = self._make_one(
+            self.BACKUP_ID,
+            instance,
+            database=self.DATABASE_NAME,
+            version_time=version_timestamp,
+        )
+
+        backup_pb = Backup(
+            database=self.DATABASE_NAME,
+            version_time=version_timestamp,
+        )
+
+        future = backup.create()
+        self.assertIs(future, op_future)
+
+        request = CreateBackupRequest(
+            parent=self.INSTANCE_NAME,
+            backup_id=self.BACKUP_ID,
+            backup=backup_pb,
+        )
+
+        api.create_backup.assert_called_once_with(
+            request=request,
+            metadata=[("google-cloud-resource-prefix", backup.name)],
+        )
 
     def test_create_database_not_set(self):
         instance = _Instance(self.INSTANCE_NAME)
@@ -413,6 +449,20 @@ def test_create_w_invalid_encryption_config(self):
         with self.assertRaises(ValueError):
             backup.create()
 
+    def test_copy_expire_time_not_set(self):
+        client = _Client()
+        client.database_admin_api = self._make_database_admin_api()
+        instance = _Instance(self.INSTANCE_NAME, client=client)
+        backup = self._make_one(
+            self.DST_BACKUP_ID,
+            instance,
+            database=self.DATABASE_NAME,
+            source_backup=self.BACKUP_ID,
+        )
+
+        with self.assertRaises(ValueError):
+            backup.create()
+
     def test_exists_grpc_error(self):
         from google.api_core.exceptions import Unknown
 

tests/unit/test_instance.py

@@ -695,6 +695,24 @@ def test_backup_factory_explicit(self):
         self.assertIs(backup._expire_time, timestamp)
         self.assertEqual(backup._encryption_config, encryption_config)
 
+    def test_backup_factory_without_expiration_time(self):
+        from google.cloud.spanner_v1.backup import Backup
+
+        client = _Client(self.PROJECT)
+        instance = self._make_one(self.INSTANCE_ID, client, self.CONFIG_NAME)
+        BACKUP_ID = "backup-id"
+        DATABASE_NAME = "database-name"
+
+        backup = instance.backup(
+            BACKUP_ID,
+            database=DATABASE_NAME,
+        )
+
+        self.assertIsInstance(backup, Backup)
+        self.assertEqual(backup.backup_id, BACKUP_ID)
+        self.assertIs(backup._instance, instance)
+        self.assertEqual(backup._database, DATABASE_NAME)
+
     def test_list_backups_defaults(self):
         from google.cloud.spanner_admin_database_v1 import Backup as BackupPB
         from google.cloud.spanner_admin_database_v1 import DatabaseAdminClient