Reviewer suggested changes.

rajatbhatta · May 16, 2023 · Mar 16, 2023 · Mar 21, 2023 · Apr 4, 2023 · Apr 12, 2023
commit 167f68bc40edd43224a12119689bcee04801a844
@@ -125,8 +125,8 @@ class Database(object):
         (Optional) database dialect for the database
     :type database_role: str or None
     :param database_role: (Optional) user-assigned database_role for the session.
-    :type drop_protection_enabled: boolean
-    :param drop_protection_enabled: (Optional) Represents whether the database
+    :type enable_drop_protection: boolean
+    :param enable_drop_protection: (Optional) Represents whether the database
         has drop protection enabled or not.
     """
 
@@ -142,7 +142,7 @@ def __init__(
         encryption_config=None,
         database_dialect=DatabaseDialect.DATABASE_DIALECT_UNSPECIFIED,
         database_role=None,
-        drop_protection_enabled=False,
+        enable_drop_protection=False,
     ):
         self.database_id = database_id
         self._instance = instance
@@ -160,7 +160,7 @@ def __init__(
         self._encryption_config = encryption_config
         self._database_dialect = database_dialect
         self._database_role = database_role
-        self.drop_protection_enabled = drop_protection_enabled
+        self._enable_drop_protection = enable_drop_protection
         self._reconciling = False
 
         if pool is None:
@@ -344,6 +344,20 @@ def reconciling(self):
         """
         return self._reconciling
 
+    @property
+    def enable_drop_protection(self):
+        """Whether the database has drop protection enabled.
+
+        :rtype: boolean
+        :returns: a boolean representing whether the database has drop
+            protection enabled
+        """
+        return self._enable_drop_protection
+
+    @enable_drop_protection.setter
+    def enable_drop_protection(self, value):
+        self._enable_drop_protection = value
+
     @property
     def logger(self):
         """Logger used by the database.
@@ -473,15 +487,15 @@ def reload(self):
         self._encryption_info = response.encryption_info
         self._default_leader = response.default_leader
         self._database_dialect = response.database_dialect
-        self.drop_protection_enabled = response.enable_drop_protection
+        self._enable_drop_protection = response.enable_drop_protection
 
     def update_ddl(self, ddl_statements, operation_id=""):
         """Update DDL for this database.
 
         Apply any configured schema from :attr:`ddl_statements`.
 
         See
-        https://cloud.google.com/spanner/reference/rpc/google.spanner.admin.database.v1#google.spanner.admin.database.v1.DatabaseAdmin.UpdateDatabase
+        https://cloud.google.com/spanner/reference/rpc/google.spanner.admin.database.v1#google.spanner.admin.database.v1.DatabaseAdmin.UpdateDatabaseDdl
 
         :type ddl_statements: Sequence[str]
         :param ddl_statements: a list of DDL statements to use on this database
@@ -505,34 +519,38 @@ def update_ddl(self, ddl_statements, operation_id=""):
         future = api.update_database_ddl(request=request, metadata=metadata)
         return future
 
-    def update(self):
+    def update(self, fields):
         """Update this database.
 
         See
-        TODO: insert documentation once ready
+        https://cloud.google.com/spanner/reference/rpc/google.spanner.admin.database.v1#google.spanner.admin.database.v1.DatabaseAdmin.UpdateDatabase
 
         .. note::
 
-            Updates the `drop_protection_enabled` field. To change this value
-            before updating, set it via
+            Updates the specified fields of a Cloud Spanner database. Currently,
+            only the `enable_drop_protection` field supports updates. To change
+            this value before updating, set it via
 
             .. code:: python
 
-                database.drop_protection_enabled = True
+                database.enable_drop_protection = True
 
            before calling :meth:`update`.
 
+        :type fields: Sequence[str]
+        :param fields: a list of fields to update
+
         :rtype: :class:`google.api_core.operation.Operation`
         :returns: an operation instance
         :raises NotFound: if the database does not exist
         """
         api = self._instance._client.database_admin_api
         database_pb = DatabasePB(
-            name=self.name, enable_drop_protection=self.drop_protection_enabled
+            name=self.name, enable_drop_protection=self._enable_drop_protection
         )
 
         # Only support updating drop protection for now.
-        field_mask = FieldMask(paths=["enable_drop_protection"])
+        field_mask = FieldMask(paths=fields)
         metadata = _metadata_with_prefix(self.name)
 
         future = api.update_database(

@@ -432,6 +432,7 @@ def database(
         encryption_config=None,
         database_dialect=DatabaseDialect.DATABASE_DIALECT_UNSPECIFIED,
         database_role=None,
+        enable_drop_protection=False,
     ):
         """Factory to create a database within this instance.
 
@@ -467,6 +468,10 @@ def database(
         :param database_dialect:
             (Optional) database dialect for the database
 
+        :type enable_drop_protection: boolean
+        :param enable_drop_protection: (Optional) Represents whether the database
+            has drop protection enabled or not.
+
         :rtype: :class:`~google.cloud.spanner_v1.database.Database`
         :returns: a database owned by this instance.
         """
@@ -479,6 +484,7 @@ def database(
             encryption_config=encryption_config,
             database_dialect=database_dialect,
             database_role=database_role,
+            enable_drop_protection=enable_drop_protection,
         )
 
     def list_databases(self, page_size=None):

@@ -563,36 +563,32 @@ def _unit_of_work(transaction, name):
 
     assert len(rows) == 2
 
+
 def test_update_database(
-    not_emulator,
-    shared_database,
-    shared_instance,
-    database_operation_timeout
+    not_emulator, shared_database, shared_instance, database_operation_timeout
 ):
-    old_protection = shared_database.drop_protection_enabled
+    old_protection = shared_database.enable_drop_protection
     new_protection = True
-    shared_database.drop_protection_enabled = new_protection
-    operation = shared_database.update()
+    shared_database.enable_drop_protection = new_protection
+    operation = shared_database.update(["enable_drop_protection"])
 
     # We want to make sure the operation completes.
     operation.result(database_operation_timeout)  # raises on failure / timeout.
 
     # Create a new database instance and reload it.
     database_alt = shared_instance.database(shared_database.name.split("/")[-1])
-    assert database_alt.drop_protection_enabled != new_protection
+    assert database_alt.enable_drop_protection != new_protection
 
     database_alt.reload()
-    assert database_alt.drop_protection_enabled == new_protection
+    assert database_alt.enable_drop_protection == new_protection
 
-    with pytest.raises(exceptions.FailedPrecondition,
-                       match='`enable_drop_protection` setting'):
+    with pytest.raises(exceptions.FailedPrecondition):
         database_alt.drop()
 
-    with pytest.raises(exceptions.FailedPrecondition,
-                       match='drop protection enabled'):
+    with pytest.raises(exceptions.FailedPrecondition):
         shared_instance.delete()
 
     # Make sure to put the database back the way it was for the
     # other test cases.
-    shared_database.drop_protection_enabled = old_protection
-    shared_database.update()
+    shared_database.enable_drop_protection = old_protection
+    shared_database.update(["enable_drop_protection"])
@@ -888,15 +888,14 @@ def test_update_success(self):
         instance = _Instance(self.INSTANCE_NAME, client=client)
         pool = _Pool()
         database = self._make_one(
-            self.DATABASE_ID, instance, drop_protection_enabled=True, pool=pool
+            self.DATABASE_ID, instance, enable_drop_protection=True, pool=pool
         )
 
-        future = database.update()
+        future = database.update(["enable_drop_protection"])
 
         self.assertIs(future, op_future)
 
-        expected_database = DatabasePB(name=database.name,
-                                       enable_drop_protection=True)
+        expected_database = DatabasePB(name=database.name, enable_drop_protection=True)
 
         field_mask = FieldMask(paths=["enable_drop_protection"])