From f60ffcb0448962a13715048abe797456c7855923 Mon Sep 17 00:00:00 2001 From: Astha Mohta Date: Thu, 11 Nov 2021 18:20:44 +0530 Subject: [PATCH 1/7] chore: changing default region to us-west1 --- samples/samples/conftest.py | 2 +- tests/system/conftest.py | 6 +++++- tests/system/test_backup_api.py | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/samples/samples/conftest.py b/samples/samples/conftest.py index b3728a4db4..d4e8a09021 100644 --- a/samples/samples/conftest.py +++ b/samples/samples/conftest.py @@ -87,7 +87,7 @@ def multi_region_instance_id(): @pytest.fixture(scope="module") def instance_config(spanner_client): return "{}/instanceConfigs/{}".format( - spanner_client.project_name, "regional-us-central1" + spanner_client.project_name, "us-west1" ) diff --git a/tests/system/conftest.py b/tests/system/conftest.py index 3a8c973f1b..7e74725189 100644 --- a/tests/system/conftest.py +++ b/tests/system/conftest.py @@ -93,7 +93,11 @@ def instance_config(instance_configs): if not instance_configs: raise ValueError("No instance configs found.") - yield instance_configs[0] + us_west1_config = [ + config for config in instance_configs if config.display_name == "us-west1" + ] + config = us_west1_config[0] if len(us_west1_config) > 0 else instance_configs[0] + yield config @pytest.fixture(scope="session") diff --git a/tests/system/test_backup_api.py b/tests/system/test_backup_api.py index de521775d4..ce77f5a161 100644 --- a/tests/system/test_backup_api.py +++ b/tests/system/test_backup_api.py @@ -52,8 +52,8 @@ def same_config_instance(spanner_client, shared_instance, instance_operation_tim @pytest.fixture(scope="session") def diff_config(shared_instance, instance_configs): current_config = shared_instance.configuration_name - for config in instance_configs: - if "-us-" in config.name and config.name != current_config: + for config in reversed(instance_configs): + if "west1" in config.name and config.name != current_config: return config.name return None From 0882048b2a5c7ad227734faa510a040be5999112 Mon Sep 17 00:00:00 2001 From: WhiteSource Renovate Date: Fri, 12 Nov 2021 03:07:26 +0100 Subject: [PATCH 2/7] chore(deps): update all dependencies (#602) --- samples/samples/requirements-test.txt | 2 +- samples/samples/requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/samples/requirements-test.txt b/samples/samples/requirements-test.txt index 151311f6cf..473151b403 100644 --- a/samples/samples/requirements-test.txt +++ b/samples/samples/requirements-test.txt @@ -1,4 +1,4 @@ pytest==6.2.5 pytest-dependency==0.5.1 mock==4.0.3 -google-cloud-testutils==1.1.0 +google-cloud-testutils==1.2.0 diff --git a/samples/samples/requirements.txt b/samples/samples/requirements.txt index a203d777f9..e37b2f24fa 100644 --- a/samples/samples/requirements.txt +++ b/samples/samples/requirements.txt @@ -1,2 +1,2 @@ -google-cloud-spanner==3.10.0 +google-cloud-spanner==3.11.1 futures==3.3.0; python_version < "3" From 33b6d969629e18eecb6c196195fdd08f2dd7fce0 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Fri, 12 Nov 2021 07:57:39 -0500 Subject: [PATCH 3/7] chore: add default_version and codeowner_team to .repo-metadata.json (#641) --- .repo-metadata.json | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/.repo-metadata.json b/.repo-metadata.json index 950a765d11..4852c16184 100644 --- a/.repo-metadata.json +++ b/.repo-metadata.json @@ -1,14 +1,16 @@ { - "name": "spanner", - "name_pretty": "Cloud Spanner", - "product_documentation": "https://cloud.google.com/spanner/docs/", - "client_documentation": "https://googleapis.dev/python/spanner/latest", - "issue_tracker": "https://issuetracker.google.com/issues?q=componentid:190851%2B%20status:open", - "release_level": "ga", - "language": "python", - "library_type": "GAPIC_COMBO", - "repo": "googleapis/python-spanner", - "distribution_name": "google-cloud-spanner", - "api_id": "spanner.googleapis.com", - "requires_billing": true -} \ No newline at end of file + "name": "spanner", + "name_pretty": "Cloud Spanner", + "product_documentation": "https://cloud.google.com/spanner/docs/", + "client_documentation": "https://googleapis.dev/python/spanner/latest", + "issue_tracker": "https://issuetracker.google.com/issues?q=componentid:190851%2B%20status:open", + "release_level": "ga", + "language": "python", + "library_type": "GAPIC_COMBO", + "repo": "googleapis/python-spanner", + "distribution_name": "google-cloud-spanner", + "api_id": "spanner.googleapis.com", + "requires_billing": true, + "default_version": "v1", + "codeowner_team": "@googleapis/api-spanner-python" +} From 415becaef7b8b29c3c9898fc6fa7c204377b22ed Mon Sep 17 00:00:00 2001 From: Ilya Gurov Date: Sat, 13 Nov 2021 11:50:28 +0300 Subject: [PATCH 4/7] feat(db_api): support stale reads (#584) --- google/cloud/spanner_dbapi/connection.py | 41 +++++- google/cloud/spanner_dbapi/cursor.py | 4 +- samples/samples/conftest.py | 4 +- tests/system/test_dbapi.py | 34 ++++- tests/unit/spanner_dbapi/test_connection.py | 130 ++++++++++++++++++-- 5 files changed, 196 insertions(+), 17 deletions(-) diff --git a/google/cloud/spanner_dbapi/connection.py b/google/cloud/spanner_dbapi/connection.py index ba9fea3858..e6d1d64db1 100644 --- a/google/cloud/spanner_dbapi/connection.py +++ b/google/cloud/spanner_dbapi/connection.py @@ -87,6 +87,7 @@ def __init__(self, instance, database, read_only=False): # connection close self._own_pool = True self._read_only = read_only + self._staleness = None @property def autocommit(self): @@ -165,6 +166,42 @@ def read_only(self, value): ) self._read_only = value + @property + def staleness(self): + """Current read staleness option value of this `Connection`. + + Returns: + dict: Staleness type and value. + """ + return self._staleness or {} + + @staleness.setter + def staleness(self, value): + """Read staleness option setter. + + Args: + value (dict): Staleness type and value. + """ + if self.inside_transaction: + raise ValueError( + "`staleness` option can't be changed while a transaction is in progress. " + "Commit or rollback the current transaction and try again." + ) + + possible_opts = ( + "read_timestamp", + "min_read_timestamp", + "max_staleness", + "exact_staleness", + ) + if value is not None and sum([opt in value for opt in possible_opts]) != 1: + raise ValueError( + "Expected one of the following staleness options: " + "read_timestamp, min_read_timestamp, max_staleness, exact_staleness." + ) + + self._staleness = value + def _session_checkout(self): """Get a Cloud Spanner session from the pool. @@ -284,7 +321,9 @@ def snapshot_checkout(self): """ if self.read_only and not self.autocommit: if not self._snapshot: - self._snapshot = Snapshot(self._session_checkout(), multi_use=True) + self._snapshot = Snapshot( + self._session_checkout(), multi_use=True, **self.staleness + ) self._snapshot.begin() return self._snapshot diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index 27303a09a6..e9e4862281 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -426,7 +426,9 @@ def _handle_DQL(self, sql, params): ) else: # execute with single-use snapshot - with self.connection.database.snapshot() as snapshot: + with self.connection.database.snapshot( + **self.connection.staleness + ) as snapshot: self._handle_DQL_with_snapshot(snapshot, sql, params) def __enter__(self): diff --git a/samples/samples/conftest.py b/samples/samples/conftest.py index d4e8a09021..478cf5f738 100644 --- a/samples/samples/conftest.py +++ b/samples/samples/conftest.py @@ -87,7 +87,7 @@ def multi_region_instance_id(): @pytest.fixture(scope="module") def instance_config(spanner_client): return "{}/instanceConfigs/{}".format( - spanner_client.project_name, "us-west1" + spanner_client.project_name, "regional-us-west1" ) @@ -201,7 +201,7 @@ def sample_database(sample_instance, database_id, database_ddl): def kms_key_name(spanner_client): return "projects/{}/locations/{}/keyRings/{}/cryptoKeys/{}".format( spanner_client.project, - "us-central1", + "us-west1", "spanner-test-keyring", "spanner-test-cmek", ) diff --git a/tests/system/test_dbapi.py b/tests/system/test_dbapi.py index 4c3989a7a4..d0ad26e79f 100644 --- a/tests/system/test_dbapi.py +++ b/tests/system/test_dbapi.py @@ -12,13 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. +import datetime import hashlib import pickle import pkg_resources import pytest from google.cloud import spanner_v1 -from google.cloud.spanner_dbapi.connection import connect, Connection +from google.cloud._helpers import UTC +from google.cloud.spanner_dbapi.connection import connect +from google.cloud.spanner_dbapi.connection import Connection from google.cloud.spanner_dbapi.exceptions import ProgrammingError from google.cloud.spanner_v1 import JsonObject from . import _helpers @@ -429,3 +432,32 @@ def test_read_only(shared_instance, dbapi_database): cur.execute("SELECT * FROM contacts") conn.commit() + + +def test_staleness(shared_instance, dbapi_database): + """Check the DB API `staleness` option.""" + conn = Connection(shared_instance, dbapi_database) + cursor = conn.cursor() + + before_insert = datetime.datetime.utcnow().replace(tzinfo=UTC) + + cursor.execute( + """ +INSERT INTO contacts (contact_id, first_name, last_name, email) +VALUES (1, 'first-name', 'last-name', 'test.email@example.com') + """ + ) + conn.commit() + + conn.read_only = True + conn.staleness = {"read_timestamp": before_insert} + cursor.execute("SELECT * FROM contacts") + conn.commit() + assert len(cursor.fetchall()) == 0 + + conn.staleness = None + cursor.execute("SELECT * FROM contacts") + conn.commit() + assert len(cursor.fetchall()) == 1 + + conn.close() diff --git a/tests/unit/spanner_dbapi/test_connection.py b/tests/unit/spanner_dbapi/test_connection.py index 34e50255f9..0eea3eaf5b 100644 --- a/tests/unit/spanner_dbapi/test_connection.py +++ b/tests/unit/spanner_dbapi/test_connection.py @@ -14,6 +14,7 @@ """Cloud Spanner DB-API Connection class unit tests.""" +import datetime import mock import unittest import warnings @@ -688,9 +689,6 @@ def test_retry_transaction_w_empty_response(self): run_mock.assert_called_with(statement, retried=True) def test_validate_ok(self): - def exit_func(self, exc_type, exc_value, traceback): - pass - connection = self._make_connection() # mock snapshot context manager @@ -699,7 +697,7 @@ def exit_func(self, exc_type, exc_value, traceback): snapshot_ctx = mock.Mock() snapshot_ctx.__enter__ = mock.Mock(return_value=snapshot_obj) - snapshot_ctx.__exit__ = exit_func + snapshot_ctx.__exit__ = exit_ctx_func snapshot_method = mock.Mock(return_value=snapshot_ctx) connection.database.snapshot = snapshot_method @@ -710,9 +708,6 @@ def exit_func(self, exc_type, exc_value, traceback): def test_validate_fail(self): from google.cloud.spanner_dbapi.exceptions import OperationalError - def exit_func(self, exc_type, exc_value, traceback): - pass - connection = self._make_connection() # mock snapshot context manager @@ -721,7 +716,7 @@ def exit_func(self, exc_type, exc_value, traceback): snapshot_ctx = mock.Mock() snapshot_ctx.__enter__ = mock.Mock(return_value=snapshot_obj) - snapshot_ctx.__exit__ = exit_func + snapshot_ctx.__exit__ = exit_ctx_func snapshot_method = mock.Mock(return_value=snapshot_ctx) connection.database.snapshot = snapshot_method @@ -734,9 +729,6 @@ def exit_func(self, exc_type, exc_value, traceback): def test_validate_error(self): from google.cloud.exceptions import NotFound - def exit_func(self, exc_type, exc_value, traceback): - pass - connection = self._make_connection() # mock snapshot context manager @@ -745,7 +737,7 @@ def exit_func(self, exc_type, exc_value, traceback): snapshot_ctx = mock.Mock() snapshot_ctx.__enter__ = mock.Mock(return_value=snapshot_obj) - snapshot_ctx.__exit__ = exit_func + snapshot_ctx.__exit__ = exit_ctx_func snapshot_method = mock.Mock(return_value=snapshot_ctx) connection.database.snapshot = snapshot_method @@ -763,3 +755,117 @@ def test_validate_closed(self): with self.assertRaises(InterfaceError): connection.validate() + + def test_staleness_invalid_value(self): + """Check that `staleness` property accepts only correct values.""" + connection = self._make_connection() + + # incorrect staleness type + with self.assertRaises(ValueError): + connection.staleness = {"something": 4} + + # no expected staleness types + with self.assertRaises(ValueError): + connection.staleness = {} + + def test_staleness_inside_transaction(self): + """ + Check that it's impossible to change the `staleness` + option if a transaction is in progress. + """ + connection = self._make_connection() + connection._transaction = mock.Mock(committed=False, rolled_back=False) + + with self.assertRaises(ValueError): + connection.staleness = {"read_timestamp": datetime.datetime(2021, 9, 21)} + + def test_staleness_multi_use(self): + """ + Check that `staleness` option is correctly + sent to the `Snapshot()` constructor. + + READ_ONLY, NOT AUTOCOMMIT + """ + timestamp = datetime.datetime(2021, 9, 20) + + connection = self._make_connection() + connection._session = "session" + connection.read_only = True + connection.staleness = {"read_timestamp": timestamp} + + with mock.patch( + "google.cloud.spanner_dbapi.connection.Snapshot" + ) as snapshot_mock: + connection.snapshot_checkout() + + snapshot_mock.assert_called_with( + "session", multi_use=True, read_timestamp=timestamp + ) + + def test_staleness_single_use_autocommit(self): + """ + Check that `staleness` option is correctly + sent to the snapshot context manager. + + NOT READ_ONLY, AUTOCOMMIT + """ + timestamp = datetime.datetime(2021, 9, 20) + + connection = self._make_connection() + connection._session_checkout = mock.MagicMock(autospec=True) + + connection.autocommit = True + connection.staleness = {"read_timestamp": timestamp} + + # mock snapshot context manager + snapshot_obj = mock.Mock() + snapshot_obj.execute_sql = mock.Mock(return_value=[1]) + + snapshot_ctx = mock.Mock() + snapshot_ctx.__enter__ = mock.Mock(return_value=snapshot_obj) + snapshot_ctx.__exit__ = exit_ctx_func + snapshot_method = mock.Mock(return_value=snapshot_ctx) + + connection.database.snapshot = snapshot_method + + cursor = connection.cursor() + cursor.execute("SELECT 1") + + connection.database.snapshot.assert_called_with(read_timestamp=timestamp) + + def test_staleness_single_use_readonly_autocommit(self): + """ + Check that `staleness` option is correctly sent to the + snapshot context manager while in `autocommit` mode. + + READ_ONLY, AUTOCOMMIT + """ + timestamp = datetime.datetime(2021, 9, 20) + + connection = self._make_connection() + connection.autocommit = True + connection.read_only = True + connection._session_checkout = mock.MagicMock(autospec=True) + + connection.staleness = {"read_timestamp": timestamp} + + # mock snapshot context manager + snapshot_obj = mock.Mock() + snapshot_obj.execute_sql = mock.Mock(return_value=[1]) + + snapshot_ctx = mock.Mock() + snapshot_ctx.__enter__ = mock.Mock(return_value=snapshot_obj) + snapshot_ctx.__exit__ = exit_ctx_func + snapshot_method = mock.Mock(return_value=snapshot_ctx) + + connection.database.snapshot = snapshot_method + + cursor = connection.cursor() + cursor.execute("SELECT 1") + + connection.database.snapshot.assert_called_with(read_timestamp=timestamp) + + +def exit_ctx_func(self, exc_type, exc_value, traceback): + """Context __exit__ method mock.""" + pass From 4b808740b6376dd2efbc3eba8f590efda101c589 Mon Sep 17 00:00:00 2001 From: Astha Mohta Date: Wed, 8 Dec 2021 21:10:56 +0530 Subject: [PATCH 5/7] feat: removing changes from samples --- samples/samples/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/samples/conftest.py b/samples/samples/conftest.py index 478cf5f738..b3728a4db4 100644 --- a/samples/samples/conftest.py +++ b/samples/samples/conftest.py @@ -87,7 +87,7 @@ def multi_region_instance_id(): @pytest.fixture(scope="module") def instance_config(spanner_client): return "{}/instanceConfigs/{}".format( - spanner_client.project_name, "regional-us-west1" + spanner_client.project_name, "regional-us-central1" ) @@ -201,7 +201,7 @@ def sample_database(sample_instance, database_id, database_ddl): def kms_key_name(spanner_client): return "projects/{}/locations/{}/keyRings/{}/cryptoKeys/{}".format( spanner_client.project, - "us-west1", + "us-central1", "spanner-test-keyring", "spanner-test-cmek", ) From ae87d326b837eb7fa9d6ee33c18ffec10e19dcfc Mon Sep 17 00:00:00 2001 From: Astha Mohta Date: Thu, 16 Dec 2021 23:04:35 +0530 Subject: [PATCH 6/7] chore: change region --- tests/system/test_backup_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/test_backup_api.py b/tests/system/test_backup_api.py index ce77f5a161..77ffca0f44 100644 --- a/tests/system/test_backup_api.py +++ b/tests/system/test_backup_api.py @@ -53,7 +53,7 @@ def same_config_instance(spanner_client, shared_instance, instance_operation_tim def diff_config(shared_instance, instance_configs): current_config = shared_instance.configuration_name for config in reversed(instance_configs): - if "west1" in config.name and config.name != current_config: + if "-us-" in config.name and config.name != current_config: return config.name return None From 7f350e389c64939569bea4453fdd8a00333fa32c Mon Sep 17 00:00:00 2001 From: Astha Mohta Date: Mon, 20 Dec 2021 21:11:59 +0530 Subject: [PATCH 7/7] fix: fix in sample list-backups --- samples/samples/backup_sample.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/samples/backup_sample.py b/samples/samples/backup_sample.py index 4b2001a0e6..d22530c735 100644 --- a/samples/samples/backup_sample.py +++ b/samples/samples/backup_sample.py @@ -198,9 +198,9 @@ def list_backup_operations(instance_id, database_id): # List the CreateBackup operations. filter_ = ( - "(metadata.database:{}) AND " "(metadata.@type:type.googleapis.com/" - "google.spanner.admin.database.v1.CreateBackupMetadata)" + "google.spanner.admin.database.v1.CreateBackupMetadata) " + "AND (metadata.database:{})" ).format(database_id) operations = instance.list_backup_operations(filter_=filter_) for op in operations: