From 6082d305f0791d95032fee1f6e3d98775dd79479 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Mon, 6 Apr 2020 20:38:07 +0000 Subject: [PATCH 01/19] feat: add support for policy tags in schema --- google/cloud/bigquery/schema.py | 100 +++++++++++++++++++++++++++++++- tests/unit/test_schema.py | 28 +++++++-- 2 files changed, 122 insertions(+), 6 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 3878a80a9..13e861790 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -62,14 +62,19 @@ class SchemaField(object): fields (Tuple[google.cloud.bigquery.schema.SchemaField]): subfields (requires ``field_type`` of 'RECORD'). + + policy_tags (Optional[PolicyTagList]): The policy tag list for the field. + """ - def __init__(self, name, field_type, mode="NULLABLE", description=None, fields=()): + def __init__(self, name, field_type, mode="NULLABLE", description=None, fields=(), policy_tags=None): self._name = name self._field_type = field_type self._mode = mode self._description = description self._fields = tuple(fields) + self._policy_tags = policy_tags + @classmethod def from_api_repr(cls, api_repr): @@ -87,12 +92,16 @@ def from_api_repr(cls, api_repr): mode = api_repr.get("mode", "NULLABLE") description = api_repr.get("description") fields = api_repr.get("fields", ()) + policy_tags = api_repr.get("policyTags") + if policy_tags is not None: + policy_tags = PolicyTagList.from_api_repr(policy_tags) return cls( field_type=api_repr["type"].upper(), fields=[cls.from_api_repr(f) for f in fields], mode=mode.upper(), description=description, name=api_repr["name"], + policy_tags=policy_tags, ) @property @@ -136,6 +145,18 @@ def fields(self): """ return self._fields + @property + def policy_tags(self): + """Optional[google.cloud.bigquery.schema.PolicyTagList]: Policy tag list + definition for this field. + + Raises: + ValueError: + if the value is not :class:`~google.cloud.bigquery.schema.PolicyTagList` + or :data:`None`. + """ + return self._policy_tags + def to_api_repr(self): """Return a dictionary representing this schema field. @@ -155,6 +176,10 @@ def to_api_repr(self): if self.field_type.upper() in _STRUCT_TYPES: answer["fields"] = [f.to_api_repr() for f in self.fields] + # If this contains a policy tag definition, include that as well: + if self.policy_tags is not None: + answer["policyTags"] = self.policy_tags.to_api_repr() + # Done; return the serialized dictionary. return answer @@ -172,6 +197,7 @@ def _key(self): self._mode.upper(), self._description, self._fields, + self._policy_tags, ) def to_standard_sql(self): @@ -244,7 +270,8 @@ def _parse_schema_resource(info): mode = r_field.get("mode", "NULLABLE") description = r_field.get("description") sub_fields = _parse_schema_resource(r_field) - schema.append(SchemaField(name, field_type, mode, description, sub_fields)) + policy_tags = r_field.get("policyTags", None) + schema.append(SchemaField(name, field_type, mode, description, sub_fields, policy_tags)) return schema @@ -291,3 +318,72 @@ def _to_schema_fields(schema): field if isinstance(field, SchemaField) else SchemaField.from_api_repr(field) for field in schema ] + +class PolicyTagList(object): + """Define Policy Tags for a column. + + Args: + names (Union[List[str], None]): list of policy tags to associate with + the column. + """ + + def __init__(self, names=None): + self._properties = {} + if names is not None: + self.names = names + + @property + def names(self): + """Union[List[str], None]: Policy tags associated with this definition. + """ + return self._properties.get("names", ()) + + @names.setter + def names(self, value): + """Union[List[str], None]: Policy tags associated with this definition. + + (Defaults to :data:`None`). + """ + if value is not None: + self._properties["names"] = value + else: + if "names" in self._properties: + del self._properties["names"] + + @classmethod + def from_api_repr(cls, api_repr): + """Return a :class:`PolicyTagList` object deserialized from a dict. + + This method creates a new ``PolicyTagList`` instance that points to + the ``api_repr`` parameter as its internal properties dict. This means + that when a ``PolicyTagList`` instance is stored as a property of + another object, any changes made at the higher level will also appear + here. + + Args: + api_repr (Mapping[str, str]): + The serialized representation of the PolicyTagList, such as + what is output by :meth:`to_api_repr`. + + Returns: + google.cloud.bigquery.schema.PolicyTagList: + The ``PolicyTagList`` object. + """ + instance = cls() + instance._properties = api_repr + return instance + + def to_api_repr(self): + """Return a dictionary representing this object. + + This method returns the properties dict of the ``PolicyTagList`` + instance rather than making a copy. This means that when a + ``PolicyTagList`` instance is stored as a property of another + object, any changes made at the higher level will also appear here. + + Returns: + dict: + A dictionary representing the PolicyTagList object in + serialized form. + """ + return self._properties \ No newline at end of file diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index e1bdd7b2f..fb77341e9 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -63,13 +63,33 @@ def test_constructor_subfields(self): self.assertIs(field._fields[0], sub_field1) self.assertIs(field._fields[1], sub_field2) + def test_constructor_with_policy_tags(self): + from google.cloud.bigquery.schema import PolicyTagList + + policy = PolicyTagList(names=("foo", "bar")) + field = self._make_one("test", "STRING", mode="REQUIRED", description="Testing", policy_tags=policy) + self.assertEqual(field._name, "test") + self.assertEqual(field._field_type, "STRING") + self.assertEqual(field._mode, "REQUIRED") + self.assertEqual(field._description, "Testing") + self.assertEqual(field._fields, ()) + self.assertEqual(field._policy_tags, policy) + def test_to_api_repr(self): - field = self._make_one("foo", "INTEGER", "NULLABLE") + from google.cloud.bigquery.schema import PolicyTagList + + policy = PolicyTagList(names=("foo", "bar")) self.assertEqual( - field.to_api_repr(), - {"mode": "NULLABLE", "name": "foo", "type": "INTEGER", "description": None}, + policy.to_api_repr(), + {"names": ("foo", "bar")} , ) + field = self._make_one("foo", "INTEGER", "NULLABLE", policy_tags=policy) + self.assertEqual( + field.to_api_repr(), + {"mode": "NULLABLE", "name": "foo", "type": "INTEGER", "description": None, "policyTags": {"names": ("foo","bar")}}, + ) + def test_to_api_repr_with_subfield(self): for record_type in ("RECORD", "STRUCT"): subfield = self._make_one("bar", "INTEGER", "NULLABLE") @@ -408,7 +428,7 @@ def test___hash__not_equals(self): def test___repr__(self): field1 = self._make_one("field1", "STRING") - expected = "SchemaField('field1', 'STRING', 'NULLABLE', None, ())" + expected = "SchemaField('field1', 'STRING', 'NULLABLE', None, (), None)" self.assertEqual(repr(field1), expected) From 88790b5d51251d9e4b2748c1524a7d302c4ec37f Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 16 Apr 2020 18:23:52 +0000 Subject: [PATCH 02/19] blacken --- google/cloud/bigquery/schema.py | 18 ++++++++++++++---- tests/unit/test_schema.py | 21 ++++++++++++++------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 13e861790..71271a4c2 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -67,7 +67,15 @@ class SchemaField(object): """ - def __init__(self, name, field_type, mode="NULLABLE", description=None, fields=(), policy_tags=None): + def __init__( + self, + name, + field_type, + mode="NULLABLE", + description=None, + fields=(), + policy_tags=None, + ): self._name = name self._field_type = field_type self._mode = mode @@ -75,7 +83,6 @@ def __init__(self, name, field_type, mode="NULLABLE", description=None, fields=( self._fields = tuple(fields) self._policy_tags = policy_tags - @classmethod def from_api_repr(cls, api_repr): """Return a ``SchemaField`` object deserialized from a dictionary. @@ -271,7 +278,9 @@ def _parse_schema_resource(info): description = r_field.get("description") sub_fields = _parse_schema_resource(r_field) policy_tags = r_field.get("policyTags", None) - schema.append(SchemaField(name, field_type, mode, description, sub_fields, policy_tags)) + schema.append( + SchemaField(name, field_type, mode, description, sub_fields, policy_tags) + ) return schema @@ -319,6 +328,7 @@ def _to_schema_fields(schema): for field in schema ] + class PolicyTagList(object): """Define Policy Tags for a column. @@ -386,4 +396,4 @@ def to_api_repr(self): A dictionary representing the PolicyTagList object in serialized form. """ - return self._properties \ No newline at end of file + return self._properties diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index fb77341e9..d09abf263 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -65,9 +65,11 @@ def test_constructor_subfields(self): def test_constructor_with_policy_tags(self): from google.cloud.bigquery.schema import PolicyTagList - + policy = PolicyTagList(names=("foo", "bar")) - field = self._make_one("test", "STRING", mode="REQUIRED", description="Testing", policy_tags=policy) + field = self._make_one( + "test", "STRING", mode="REQUIRED", description="Testing", policy_tags=policy + ) self.assertEqual(field._name, "test") self.assertEqual(field._field_type, "STRING") self.assertEqual(field._mode, "REQUIRED") @@ -77,19 +79,24 @@ def test_constructor_with_policy_tags(self): def test_to_api_repr(self): from google.cloud.bigquery.schema import PolicyTagList - + policy = PolicyTagList(names=("foo", "bar")) self.assertEqual( - policy.to_api_repr(), - {"names": ("foo", "bar")} , + policy.to_api_repr(), {"names": ("foo", "bar")}, ) field = self._make_one("foo", "INTEGER", "NULLABLE", policy_tags=policy) self.assertEqual( field.to_api_repr(), - {"mode": "NULLABLE", "name": "foo", "type": "INTEGER", "description": None, "policyTags": {"names": ("foo","bar")}}, + { + "mode": "NULLABLE", + "name": "foo", + "type": "INTEGER", + "description": None, + "policyTags": {"names": ("foo", "bar")}, + }, ) - + def test_to_api_repr_with_subfield(self): for record_type in ("RECORD", "STRUCT"): subfield = self._make_one("bar", "INTEGER", "NULLABLE") From d756cf2d1276b41c7da46b3744e4da8bc5579f22 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 16 Apr 2020 20:37:17 +0000 Subject: [PATCH 03/19] add more unit coverage --- tests/unit/test_schema.py | 42 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index d09abf263..e0efa37fb 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -138,6 +138,24 @@ def test_from_api_repr(self): self.assertEqual(field.fields[0].field_type, "INTEGER") self.assertEqual(field.fields[0].mode, "NULLABLE") + def test_from_api_repr_policy(self): + field = self._get_target_class().from_api_repr( + { + "fields": [{"mode": "nullable", "name": "bar", "type": "integer"}], + "name": "foo", + "type": "record", + "policyTags": ("one", "two"), + } + ) + self.assertEqual(field.name, "foo") + self.assertEqual(field.field_type, "RECORD") + self.assertEqual(field.description, "test_description") + self.assertEqual(len(field.fields), 1) + self.assertEqual(field.fields[0].name, "bar") + self.assertEqual(field.fields[0].field_type, "INTEGER") + self.assertEqual(field.fields[0].mode, "NULLABLE") + self.assertEqual(field.policy_tags, ("one", "two")) + def test_from_api_repr_defaults(self): field = self._get_target_class().from_api_repr( {"name": "foo", "type": "record"} @@ -659,3 +677,27 @@ def test_valid_mapping_representation(self): result = self._call_fut(schema) self.assertEqual(result, expected_schema) + + +class TestPolicyTags(unittest.TestCase): + @staticmethod + def _get_target_class(): + from google.cloud.bigquery.schema import PolicyTagList + + return PolicyTagList + + def _make_one(self, *args, **kw): + return self._get_target_class()(*args, **kw) + + def test_constructor(self): + empty_policy_tags = self._make_one() + self.assertIsNotNone(empty_policy_tags.names) + self.assertEqual(len(empty_policy_tags.names), 0) + policy_tags = self._make_one(("foo", "bar")) + self.assertEqual(policy_tags.names, ("foo", "bar")) + + def test_from_api_repr(self): + klass = self._get_target_class() + api_repr = {"names": ("foo")} + policy_tags = klass.from_api_repr(api_repr) + self.assertEqual(policy_tags.to_api_repr(), api_repr) From ae04022ebe18daa1e8fd660993188ea66884fd30 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 16 Apr 2020 21:21:12 +0000 Subject: [PATCH 04/19] more test cleanup --- tests/unit/test_schema.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index e0efa37fb..093167514 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -144,17 +144,16 @@ def test_from_api_repr_policy(self): "fields": [{"mode": "nullable", "name": "bar", "type": "integer"}], "name": "foo", "type": "record", - "policyTags": ("one", "two"), + "policyTags": {"names": ("one", "two")}, } ) self.assertEqual(field.name, "foo") self.assertEqual(field.field_type, "RECORD") - self.assertEqual(field.description, "test_description") + self.assertEqual(field.policy_tags.names, ("one", "two")) self.assertEqual(len(field.fields), 1) self.assertEqual(field.fields[0].name, "bar") self.assertEqual(field.fields[0].field_type, "INTEGER") self.assertEqual(field.fields[0].mode, "NULLABLE") - self.assertEqual(field.policy_tags, ("one", "two")) def test_from_api_repr_defaults(self): field = self._get_target_class().from_api_repr( From ab27d788ddc7eefb189356d04d3595ae8e7e061e Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 16 Apr 2020 23:55:04 +0000 Subject: [PATCH 05/19] more tests --- google/cloud/bigquery/schema.py | 1 + tests/unit/test_schema.py | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 71271a4c2..ef773bda1 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -355,6 +355,7 @@ def names(self, value): (Defaults to :data:`None`). """ if value is not None: + prop = self._properties.setdefault("names", ()) self._properties["names"] = value else: if "names" in self._properties: diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index 093167514..db61cf627 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -700,3 +700,11 @@ def test_from_api_repr(self): api_repr = {"names": ("foo")} policy_tags = klass.from_api_repr(api_repr) self.assertEqual(policy_tags.to_api_repr(), api_repr) + + def test_setter(self): + policy_tags = self._make_one() + self.assertEqual(len(policy_tags.names), 0) + policy_tags.names = ("foo", "bar") + self.assertEqual(policy_tags.names, ("foo", "bar")) + policy_tags.names = None + self.assertEqual(policy_tags.names, ()) From dc7170c667872a4a26cdd0105a73015f24659a7a Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Fri, 17 Apr 2020 05:17:18 +0000 Subject: [PATCH 06/19] formatting --- google/cloud/bigquery/schema.py | 1 - tests/unit/test_schema.py | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index ef773bda1..71271a4c2 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -355,7 +355,6 @@ def names(self, value): (Defaults to :data:`None`). """ if value is not None: - prop = self._properties.setdefault("names", ()) self._properties["names"] = value else: if "names" in self._properties: diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index db61cf627..d5bc5f369 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -701,6 +701,12 @@ def test_from_api_repr(self): policy_tags = klass.from_api_repr(api_repr) self.assertEqual(policy_tags.to_api_repr(), api_repr) + def test_to_api_repr(self): + taglist = self._make_one(names=("foo", "bar")) + self.assertEqual( + taglist.to_api_repr(), {"names": ("foo", "bar")}, + ) + def test_setter(self): policy_tags = self._make_one() self.assertEqual(len(policy_tags.names), 0) From c06d4a655a944dd174b23f1701b79fd8d4b0abe3 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Fri, 17 Apr 2020 18:32:08 +0000 Subject: [PATCH 07/19] more testing of names setter --- tests/unit/test_schema.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index d5bc5f369..ebad2621a 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -709,7 +709,9 @@ def test_to_api_repr(self): def test_setter(self): policy_tags = self._make_one() - self.assertEqual(len(policy_tags.names), 0) + self.assertEqual(policy_tags.names, ()) + policy_tags.names = None + self.assertEqual(policy_tags.names, ()) policy_tags.names = ("foo", "bar") self.assertEqual(policy_tags.names, ("foo", "bar")) policy_tags.names = None From 3bd1e7fcee0063490006752e47e409741e88752d Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Mon, 20 Apr 2020 20:54:55 +0000 Subject: [PATCH 08/19] address reviewer comments --- google/cloud/bigquery/schema.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 71271a4c2..50cd9fa7e 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -14,6 +14,8 @@ """Schemas for BigQuery tables / queries.""" +import copy + from six.moves import collections_abc from google.cloud.bigquery_v2 import types @@ -159,7 +161,7 @@ def policy_tags(self): Raises: ValueError: - if the value is not :class:`~google.cloud.bigquery.schema.PolicyTagList` + If the value is not :class:`~google.cloud.bigquery.schema.PolicyTagList` or :data:`None`. """ return self._policy_tags @@ -277,7 +279,7 @@ def _parse_schema_resource(info): mode = r_field.get("mode", "NULLABLE") description = r_field.get("description") sub_fields = _parse_schema_resource(r_field) - policy_tags = r_field.get("policyTags", None) + policy_tags = r_field.get("policyTags") schema.append( SchemaField(name, field_type, mode, description, sub_fields, policy_tags) ) @@ -333,24 +335,23 @@ class PolicyTagList(object): """Define Policy Tags for a column. Args: - names (Union[List[str], None]): list of policy tags to associate with + names (Union[Tuple[str], None]): list of policy tags to associate with the column. """ def __init__(self, names=None): self._properties = {} - if names is not None: - self.names = names + self.names = names @property def names(self): - """Union[List[str], None]: Policy tags associated with this definition. + """Tuple[str]: Policy tags associated with this definition. """ return self._properties.get("names", ()) @names.setter def names(self, value): - """Union[List[str], None]: Policy tags associated with this definition. + """Union[Tuple[str], None]: Policy tags associated with this definition. (Defaults to :data:`None`). """ @@ -380,7 +381,7 @@ def from_api_repr(cls, api_repr): The ``PolicyTagList`` object. """ instance = cls() - instance._properties = api_repr + instance._properties = copy.deepcopy(api_repr) return instance def to_api_repr(self): From b71b87195f0cf9f36645b3e67baa0ba26333f706 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Mon, 20 Apr 2020 20:57:21 +0000 Subject: [PATCH 09/19] docstrings migrate from unions -> optional --- google/cloud/bigquery/schema.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 50cd9fa7e..4377160b1 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -335,7 +335,7 @@ class PolicyTagList(object): """Define Policy Tags for a column. Args: - names (Union[Tuple[str], None]): list of policy tags to associate with + names (Optional[Tuple[str]]): list of policy tags to associate with the column. """ @@ -351,7 +351,7 @@ def names(self): @names.setter def names(self, value): - """Union[Tuple[str], None]: Policy tags associated with this definition. + """Optional[Tuple[str]]: Policy tags associated with this definition. (Defaults to :data:`None`). """ From 1d704c56820537ed72b6976e18099963b361516c Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 30 Apr 2020 19:23:11 +0000 Subject: [PATCH 10/19] stashing changes --- google/cloud/bigquery/schema.py | 22 ++++++++++---- tests/system.py | 51 +++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 4377160b1..90c75e835 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -101,16 +101,14 @@ def from_api_repr(cls, api_repr): mode = api_repr.get("mode", "NULLABLE") description = api_repr.get("description") fields = api_repr.get("fields", ()) - policy_tags = api_repr.get("policyTags") - if policy_tags is not None: - policy_tags = PolicyTagList.from_api_repr(policy_tags) + return cls( field_type=api_repr["type"].upper(), fields=[cls.from_api_repr(f) for f in fields], mode=mode.upper(), description=description, name=api_repr["name"], - policy_tags=policy_tags, + policy_tags=PolicyTagList.from_api_repr(api_repr.get("policyTags")), ) @property @@ -279,7 +277,7 @@ def _parse_schema_resource(info): mode = r_field.get("mode", "NULLABLE") description = r_field.get("description") sub_fields = _parse_schema_resource(r_field) - policy_tags = r_field.get("policyTags") + policy_tags = PolicyTagList.from_api_repr(r_field.get("policyTags")) schema.append( SchemaField(name, field_type, mode, description, sub_fields, policy_tags) ) @@ -356,11 +354,20 @@ def names(self, value): (Defaults to :data:`None`). """ if value is not None: - self._properties["names"] = value + self._properties["names"] = tuple(value) else: if "names" in self._properties: del self._properties["names"] + def __eq__(self, other): + if isinstance(other, self.__class__): + return self.__dict__ == other.__dict__ + else: + return False + + def __ne__(self, other): + return not self.__eq__(other) + @classmethod def from_api_repr(cls, api_repr): """Return a :class:`PolicyTagList` object deserialized from a dict. @@ -382,6 +389,9 @@ def from_api_repr(cls, api_repr): """ instance = cls() instance._properties = copy.deepcopy(api_repr) + # ensure the representation is immutable + if instance._properties is not None: + instance.names = instance.names return instance def to_api_repr(self): diff --git a/tests/system.py b/tests/system.py index 98a1edaa5..7744fdfdd 100644 --- a/tests/system.py +++ b/tests/system.py @@ -335,6 +335,57 @@ def test_create_table(self): self.assertTrue(_table_exists(table)) self.assertEqual(table.table_id, table_id) + def test_create_table_with_policy(self): + from google.cloud.bigquery.schema import PolicyTagList + + dataset = self.temp_dataset(_make_dataset_id("create_table_with_policy")) + table_id = "test_table" + policy_1 = PolicyTagList( + names=( + "projects/{}/locations/us/taxonomies/1/policyTags/2".format( + Config.CLIENT.project + ), + ) + ) + policy_2 = PolicyTagList( + names=( + "projects/{}/locations/us/taxonomies/3/policyTags/4".format( + Config.CLIENT.project + ), + ) + ) + + schema = [ + bigquery.SchemaField("full_name", "STRING", mode="REQUIRED"), + bigquery.SchemaField( + "secret_int", "INTEGER", mode="REQUIRED", policy_tags=policy_1 + ), + ] + table_arg = Table(dataset.table(table_id), schema=schema) + self.assertFalse(_table_exists(table_arg)) + + table = retry_403(Config.CLIENT.create_table)(table_arg) + self.to_delete.insert(0, table) + + self.assertTrue(_table_exists(table)) + self.assertEqual(policy_1, table.schema[1].policy_tags) + + # Amend the schema to replace the policy tags + new_schema = table.schema[:] + old_field = table.schema[1] + new_schema[1] = bigquery.SchemaField( + name=old_field.name, + field_type=old_field.field_type, + mode=old_field.mode, + description=old_field.description, + fields=old_field.fields, + policy_tags=policy_2, + ) + + table.schema = new_schema + table2 = Config.CLIENT.update_table(table, ["schema",]) + self.assertEqual(policy_2, table.schema[1].policy_tags) + def test_create_table_w_time_partitioning_w_clustering_fields(self): from google.cloud.bigquery.table import TimePartitioning from google.cloud.bigquery.table import TimePartitioningType From 9cebe40e72e666da5c7465baa9e86b5140befbe1 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 14 May 2020 19:09:27 +0000 Subject: [PATCH 11/19] revision to list-based representation, update tests --- google/cloud/bigquery/schema.py | 18 +++++++++--------- tests/system.py | 4 ++-- tests/unit/test_schema.py | 14 +++++++------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 90c75e835..02154e5c8 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -333,8 +333,9 @@ class PolicyTagList(object): """Define Policy Tags for a column. Args: - names (Optional[Tuple[str]]): list of policy tags to associate with - the column. + names (Optional[List[str]]): list of policy tags to associate with + the column. Policy tag identifiers are of the form + `projects/*/locations/*/taxonomies/*/policyTags/*`. """ def __init__(self, names=None): @@ -343,18 +344,18 @@ def __init__(self, names=None): @property def names(self): - """Tuple[str]: Policy tags associated with this definition. + """List[str]: Policy tags associated with this definition. """ - return self._properties.get("names", ()) + return self._properties.get("names", []) @names.setter def names(self, value): - """Optional[Tuple[str]]: Policy tags associated with this definition. + """Optional[List[str]]: Policy tags associated with this definition. (Defaults to :data:`None`). """ if value is not None: - self._properties["names"] = tuple(value) + self._properties["names"] = value else: if "names" in self._properties: del self._properties["names"] @@ -387,11 +388,10 @@ def from_api_repr(cls, api_repr): google.cloud.bigquery.schema.PolicyTagList: The ``PolicyTagList`` object. """ + if api_repr is None: + return None instance = cls() instance._properties = copy.deepcopy(api_repr) - # ensure the representation is immutable - if instance._properties is not None: - instance.names = instance.names return instance def to_api_repr(self): diff --git a/tests/system.py b/tests/system.py index 7744fdfdd..c0a11e380 100644 --- a/tests/system.py +++ b/tests/system.py @@ -341,11 +341,11 @@ def test_create_table_with_policy(self): dataset = self.temp_dataset(_make_dataset_id("create_table_with_policy")) table_id = "test_table" policy_1 = PolicyTagList( - names=( + names=[ "projects/{}/locations/us/taxonomies/1/policyTags/2".format( Config.CLIENT.project ), - ) + ] ) policy_2 = PolicyTagList( names=( diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index ebad2621a..e2b59bb16 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -144,12 +144,12 @@ def test_from_api_repr_policy(self): "fields": [{"mode": "nullable", "name": "bar", "type": "integer"}], "name": "foo", "type": "record", - "policyTags": {"names": ("one", "two")}, + "policyTags": {"names": ["one", "two"]}, } ) self.assertEqual(field.name, "foo") self.assertEqual(field.field_type, "RECORD") - self.assertEqual(field.policy_tags.names, ("one", "two")) + self.assertEqual(field.policy_tags.names, ["one", "two"]) self.assertEqual(len(field.fields), 1) self.assertEqual(field.fields[0].name, "bar") self.assertEqual(field.fields[0].field_type, "INTEGER") @@ -709,10 +709,10 @@ def test_to_api_repr(self): def test_setter(self): policy_tags = self._make_one() - self.assertEqual(policy_tags.names, ()) + self.assertEqual(policy_tags.names, []) policy_tags.names = None - self.assertEqual(policy_tags.names, ()) - policy_tags.names = ("foo", "bar") - self.assertEqual(policy_tags.names, ("foo", "bar")) + self.assertEqual(policy_tags.names, []) + policy_tags.names = ["foo", "bar"] + self.assertEqual(policy_tags.names, ["foo", "bar"]) policy_tags.names = None - self.assertEqual(policy_tags.names, ()) + self.assertEqual(policy_tags.names, []) From 51722a54e4b9452f553dcd7497c6a98578a8bf28 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 14 May 2020 23:05:44 +0000 Subject: [PATCH 12/19] changes to equality and testing, towards satisfying coverage --- google/cloud/bigquery/schema.py | 25 ++++++++++++++++++++----- tests/system.py | 8 ++++---- tests/unit/test_schema.py | 21 ++++++++++++++++----- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 02154e5c8..3f0c41241 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -360,14 +360,29 @@ def names(self, value): if "names" in self._properties: del self._properties["names"] + def _key(self): + """A tuple key that uniquely describes this PolicyTagList. + + Used to compute this instance's hashcode and evaluate equality. + + Returns: + Tuple: The contents of this :class:`~google.cloud.bigquery.schema.PolicyTagList`. + """ + return tuple(sorted(self._properties.items())) + def __eq__(self, other): - if isinstance(other, self.__class__): - return self.__dict__ == other.__dict__ - else: - return False + if not isinstance(other, PolicyTagList): + return NotImplemented + return self._key() == other._key() def __ne__(self, other): - return not self.__eq__(other) + return not self == other + + def __hash__(self): + return hash(self._key()) + + def __repr__(self): + return "PolicyTagList{}".format(self._key()) @classmethod def from_api_repr(cls, api_repr): diff --git a/tests/system.py b/tests/system.py index 564cd082e..dfc9d65fe 100644 --- a/tests/system.py +++ b/tests/system.py @@ -352,11 +352,11 @@ def test_create_table_with_policy(self): ] ) policy_2 = PolicyTagList( - names=( + names=[ "projects/{}/locations/us/taxonomies/3/policyTags/4".format( Config.CLIENT.project ), - ) + ] ) schema = [ @@ -387,8 +387,8 @@ def test_create_table_with_policy(self): ) table.schema = new_schema - table2 = Config.CLIENT.update_table(table, ["schema",]) - self.assertEqual(policy_2, table.schema[1].policy_tags) + table2 = Config.CLIENT.update_table(table, ["schema", ]) + self.assertEqual(policy_2, table2.schema[1].policy_tags) def test_create_table_w_time_partitioning_w_clustering_fields(self): from google.cloud.bigquery.table import TimePartitioning diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index e2b59bb16..f0e606b55 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -692,19 +692,19 @@ def test_constructor(self): empty_policy_tags = self._make_one() self.assertIsNotNone(empty_policy_tags.names) self.assertEqual(len(empty_policy_tags.names), 0) - policy_tags = self._make_one(("foo", "bar")) - self.assertEqual(policy_tags.names, ("foo", "bar")) + policy_tags = self._make_one(["foo", "bar"]) + self.assertEqual(policy_tags.names, ["foo", "bar"]) def test_from_api_repr(self): klass = self._get_target_class() - api_repr = {"names": ("foo")} + api_repr = {"names": ["foo"]} policy_tags = klass.from_api_repr(api_repr) self.assertEqual(policy_tags.to_api_repr(), api_repr) def test_to_api_repr(self): - taglist = self._make_one(names=("foo", "bar")) + taglist = self._make_one(names=["foo", "bar"]) self.assertEqual( - taglist.to_api_repr(), {"names": ("foo", "bar")}, + taglist.to_api_repr(), {"names": ["foo", "bar"]}, ) def test_setter(self): @@ -716,3 +716,14 @@ def test_setter(self): self.assertEqual(policy_tags.names, ["foo", "bar"]) policy_tags.names = None self.assertEqual(policy_tags.names, []) + + def test___eq___wrong_type(self): + policy = self._make_one(names=["foo", ]) + other = object() + self.assertNotEqual(policy, other) + self.assertEqual(policy, mock.ANY) + + def test___eq___names_mismatch(self): + policy = self._make_one(names=["foo", "bar"]) + other = self._make_one(names=["bar", "baz"]) + self.assertNotEqual(policy, other) \ No newline at end of file From c55fc7b2ba7d3304f6fe4822d8e5075cdee86a9a Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Fri, 15 May 2020 00:25:49 +0000 Subject: [PATCH 13/19] cleanup --- google/cloud/bigquery/schema.py | 5 +---- tests/system.py | 2 +- tests/unit/test_schema.py | 4 ++-- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 3f0c41241..0ec65c580 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -360,7 +360,7 @@ def names(self, value): if "names" in self._properties: del self._properties["names"] - def _key(self): + def _key(self): """A tuple key that uniquely describes this PolicyTagList. Used to compute this instance's hashcode and evaluate equality. @@ -378,9 +378,6 @@ def __eq__(self, other): def __ne__(self, other): return not self == other - def __hash__(self): - return hash(self._key()) - def __repr__(self): return "PolicyTagList{}".format(self._key()) diff --git a/tests/system.py b/tests/system.py index dfc9d65fe..49e45c772 100644 --- a/tests/system.py +++ b/tests/system.py @@ -387,7 +387,7 @@ def test_create_table_with_policy(self): ) table.schema = new_schema - table2 = Config.CLIENT.update_table(table, ["schema", ]) + table2 = Config.CLIENT.update_table(table, ["schema"]) self.assertEqual(policy_2, table2.schema[1].policy_tags) def test_create_table_w_time_partitioning_w_clustering_fields(self): diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index f0e606b55..202cbc657 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -718,7 +718,7 @@ def test_setter(self): self.assertEqual(policy_tags.names, []) def test___eq___wrong_type(self): - policy = self._make_one(names=["foo", ]) + policy = self._make_one(names=["foo"]) other = object() self.assertNotEqual(policy, other) self.assertEqual(policy, mock.ANY) @@ -726,4 +726,4 @@ def test___eq___wrong_type(self): def test___eq___names_mismatch(self): policy = self._make_one(names=["foo", "bar"]) other = self._make_one(names=["bar", "baz"]) - self.assertNotEqual(policy, other) \ No newline at end of file + self.assertNotEqual(policy, other) From 1173bb4b2ccd2aa822fe8dbba333b34073f3e5ef Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Fri, 15 May 2020 01:27:20 +0000 Subject: [PATCH 14/19] return copy --- google/cloud/bigquery/schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 0ec65c580..d6325a9a7 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -419,4 +419,4 @@ def to_api_repr(self): A dictionary representing the PolicyTagList object in serialized form. """ - return self._properties + return copy.deepcopy(self._properties) From 3febd454831c101dad5b2a21bd10fdbb3da9585d Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Fri, 15 May 2020 16:56:27 +0000 Subject: [PATCH 15/19] address api repr feedback --- google/cloud/bigquery/schema.py | 4 ++-- tests/unit/test_schema.py | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index d6325a9a7..550c2b6a6 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -397,8 +397,8 @@ def from_api_repr(cls, api_repr): what is output by :meth:`to_api_repr`. Returns: - google.cloud.bigquery.schema.PolicyTagList: - The ``PolicyTagList`` object. + Optional[google.cloud.bigquery.schema.PolicyTagList]: + The ``PolicyTagList`` object or None. """ if api_repr is None: return None diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index 202cbc657..f3168b2e0 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -701,6 +701,11 @@ def test_from_api_repr(self): policy_tags = klass.from_api_repr(api_repr) self.assertEqual(policy_tags.to_api_repr(), api_repr) + # Ensure the None case correctly returns None, rather + # than an empty instance. + policy_tags2 = klass.from_api_repr(None) + self.assertIsNone(policy_tags2) + def test_to_api_repr(self): taglist = self._make_one(names=["foo", "bar"]) self.assertEqual( From 68af6fee0355d46f7a93b10c78f84e04e16311dc Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Fri, 15 May 2020 17:39:45 +0000 Subject: [PATCH 16/19] make PolicyTagList fully immutable --- google/cloud/bigquery/schema.py | 38 +++++++++++++-------------------- tests/unit/test_schema.py | 36 +++++++++++++++++++------------ 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 550c2b6a6..247a507c5 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -14,8 +14,6 @@ """Schemas for BigQuery tables / queries.""" -import copy - from six.moves import collections_abc from google.cloud.bigquery_v2 import types @@ -333,32 +331,21 @@ class PolicyTagList(object): """Define Policy Tags for a column. Args: - names (Optional[List[str]]): list of policy tags to associate with + names ( + Optional[List[str]]): list of policy tags to associate with the column. Policy tag identifiers are of the form `projects/*/locations/*/taxonomies/*/policyTags/*`. """ - def __init__(self, names=None): + def __init__(self, names=()): self._properties = {} - self.names = names + self._properties["names"] = tuple(names) @property def names(self): - """List[str]: Policy tags associated with this definition. + """Tuple[str]: Policy tags associated with this definition. """ - return self._properties.get("names", []) - - @names.setter - def names(self, value): - """Optional[List[str]]: Policy tags associated with this definition. - - (Defaults to :data:`None`). - """ - if value is not None: - self._properties["names"] = value - else: - if "names" in self._properties: - del self._properties["names"] + return self._properties.get("names", ()) def _key(self): """A tuple key that uniquely describes this PolicyTagList. @@ -378,6 +365,9 @@ def __eq__(self, other): def __ne__(self, other): return not self == other + def __hash__(self): + return hash(self._key()) + def __repr__(self): return "PolicyTagList{}".format(self._key()) @@ -402,9 +392,8 @@ def from_api_repr(cls, api_repr): """ if api_repr is None: return None - instance = cls() - instance._properties = copy.deepcopy(api_repr) - return instance + names = api_repr.get("names", ()) + return cls(names=names) def to_api_repr(self): """Return a dictionary representing this object. @@ -419,4 +408,7 @@ def to_api_repr(self): A dictionary representing the PolicyTagList object in serialized form. """ - return copy.deepcopy(self._properties) + answer = {} + if self.names is not None: + answer["names"] = [name for name in self.names] + return answer diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index f3168b2e0..9f7ee7bb3 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -82,7 +82,7 @@ def test_to_api_repr(self): policy = PolicyTagList(names=("foo", "bar")) self.assertEqual( - policy.to_api_repr(), {"names": ("foo", "bar")}, + policy.to_api_repr(), {"names": ["foo", "bar"]}, ) field = self._make_one("foo", "INTEGER", "NULLABLE", policy_tags=policy) @@ -93,7 +93,7 @@ def test_to_api_repr(self): "name": "foo", "type": "INTEGER", "description": None, - "policyTags": {"names": ("foo", "bar")}, + "policyTags": {"names": ["foo", "bar"]}, }, ) @@ -149,7 +149,7 @@ def test_from_api_repr_policy(self): ) self.assertEqual(field.name, "foo") self.assertEqual(field.field_type, "RECORD") - self.assertEqual(field.policy_tags.names, ["one", "two"]) + self.assertEqual(field.policy_tags.names, ("one", "two")) self.assertEqual(len(field.fields), 1) self.assertEqual(field.fields[0].name, "bar") self.assertEqual(field.fields[0].field_type, "INTEGER") @@ -693,7 +693,7 @@ def test_constructor(self): self.assertIsNotNone(empty_policy_tags.names) self.assertEqual(len(empty_policy_tags.names), 0) policy_tags = self._make_one(["foo", "bar"]) - self.assertEqual(policy_tags.names, ["foo", "bar"]) + self.assertEqual(policy_tags.names, ("foo", "bar")) def test_from_api_repr(self): klass = self._get_target_class() @@ -711,16 +711,10 @@ def test_to_api_repr(self): self.assertEqual( taglist.to_api_repr(), {"names": ["foo", "bar"]}, ) - - def test_setter(self): - policy_tags = self._make_one() - self.assertEqual(policy_tags.names, []) - policy_tags.names = None - self.assertEqual(policy_tags.names, []) - policy_tags.names = ["foo", "bar"] - self.assertEqual(policy_tags.names, ["foo", "bar"]) - policy_tags.names = None - self.assertEqual(policy_tags.names, []) + taglist2 = self._make_one(names=("foo", "bar")) + self.assertEqual( + taglist2.to_api_repr(), {"names": ["foo", "bar"]}, + ) def test___eq___wrong_type(self): policy = self._make_one(names=["foo"]) @@ -732,3 +726,17 @@ def test___eq___names_mismatch(self): policy = self._make_one(names=["foo", "bar"]) other = self._make_one(names=["bar", "baz"]) self.assertNotEqual(policy, other) + + def test___hash__set_equality(self): + policy1 = self._make_one(["foo", "bar"]) + policy2 = self._make_one(["bar", "baz"]) + set_one = {policy1, policy2} + set_two = {policy1, policy2} + self.assertEqual(set_one, set_two) + + def test___hash__not_equals(self): + policy1 = self._make_one(["foo", "bar"]) + policy2 = self._make_one(["bar", "baz"]) + set_one = {policy1} + set_two = {policy2} + self.assertNotEqual(set_one, set_two) From c7b06520dbfa47d92951954b9def025f38ac33c4 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Fri, 15 May 2020 17:59:54 +0000 Subject: [PATCH 17/19] update docstring --- google/cloud/bigquery/schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 247a507c5..4538e97f6 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -332,7 +332,7 @@ class PolicyTagList(object): Args: names ( - Optional[List[str]]): list of policy tags to associate with + Optional[Tuple[str]]): list of policy tags to associate with the column. Policy tag identifiers are of the form `projects/*/locations/*/taxonomies/*/policyTags/*`. """ From fe8061e37dec50ada647fe3a52347eea1ffb2f80 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Fri, 15 May 2020 19:39:18 +0000 Subject: [PATCH 18/19] simplify to_api_repr --- google/cloud/bigquery/schema.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index 4538e97f6..eb0248dde 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -408,7 +408,5 @@ def to_api_repr(self): A dictionary representing the PolicyTagList object in serialized form. """ - answer = {} - if self.names is not None: - answer["names"] = [name for name in self.names] + answer = {"names": [name for name in self.names]} return answer From 79d90f87e996a533df5bdbb9f83c53b7ae60e56e Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Mon, 18 May 2020 16:48:17 +0000 Subject: [PATCH 19/19] remove stale doc comments --- google/cloud/bigquery/schema.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/google/cloud/bigquery/schema.py b/google/cloud/bigquery/schema.py index eb0248dde..0eaf1201b 100644 --- a/google/cloud/bigquery/schema.py +++ b/google/cloud/bigquery/schema.py @@ -154,11 +154,6 @@ def fields(self): def policy_tags(self): """Optional[google.cloud.bigquery.schema.PolicyTagList]: Policy tag list definition for this field. - - Raises: - ValueError: - If the value is not :class:`~google.cloud.bigquery.schema.PolicyTagList` - or :data:`None`. """ return self._policy_tags