Add support for EncryptionConfiguration #4845

vitaliili · 2018-02-06T22:23:33Z

As part of a query, load job, or create table request, you can specify the encryption configuration, which just consists of a single string field containing the KMS key to use.

bigquery/google/cloud/bigquery/job.py


+    @property
+    def kms_key_name(self):
+        """See


bigquery/google/cloud/bigquery/job.py

        if config.skip_leading_rows is None:
            del config.skip_leading_rows
+        if 'destinationEncryptionConfiguration' in resource:
+            key = resource['destinationEncryptionConfiguration']['kmsKeyName']


bigquery/google/cloud/bigquery/job.py

+    @kms_key_name.setter
+    def kms_key_name(self, value):
+        if not isinstance(value, six.string_types) and value is not None:
+            raise ValueError("Pass a string, or None")


bigquery/google/cloud/bigquery/job.py


+    @property
+    def kms_key_name(self):
+        """See


bigquery/google/cloud/bigquery/job.py

        config = cls()
        config._properties = copy.deepcopy(resource)
+        if 'destinationEncryptionConfiguration' in resource:
+            key = resource['destinationEncryptionConfiguration']['kmsKeyName']


bigquery/google/cloud/bigquery/job.py

                config._properties[prop] = from_resource(nested_resource)

+        if 'destinationEncryptionConfiguration' in resource:
+            key = resource['destinationEncryptionConfiguration']['kmsKeyName']


bigquery/google/cloud/bigquery/table.py

        table = cls(dataset_ref.table(table_id))
        table._set_properties(resource)
+        if 'encryptionConfiguration' in resource:
+            key = resource['encryptionConfiguration']['kmsKeyName']


bigquery/tests/unit/test_job.py

+        klass = self._get_target_class()
+        job = klass.from_api_repr(RESOURCE, client=client)
+        self.assertIs(job._client, client)
+        self._verifyResourceProperties(job, RESOURCE)


tswast · 2018-02-07T21:27:13Z

@vitaliili the documentation build is failing. There's some info in https://github.com/GoogleCloudPlatform/google-cloud-python/blob/master/CONTRIBUTING.rst to get your dev environment set up.

The way to build the docs locally is to run nox -s docs from the root directory of the repo.

bigquery/google/cloud/bigquery/job.py

        config.skip_leading_rows = _int_or_none(slr)
        if config.skip_leading_rows is None:
            del config.skip_leading_rows
+        if ('destinationEncryptionConfiguration' in resource


bigquery/google/cloud/bigquery/job.py


    def __init__(self):
        self._properties = {}
+        self._kms_key_name = None


bigquery/google/cloud/bigquery/job.py

+
+    @kms_key_name.setter
+    def kms_key_name(self, value):
+        if not isinstance(value, six.string_types) and value is not None:


tswast

Unapproving, since I do agree with Jon's comments.

Include EncryptionConfiguration in BigQuery module. Use _properties for EncryptionConfiguration implementation.

Note: I use _properties and kwargs because we want to be able to explicitly set kmsKeyName to None to allow unsetting it.

Also, fix links to job configs for destination_encryption_configurations.

tswast

Thanks @vitaliili, I've made a few additional changes to the docstrings and EncryptionConfiguration class.

@jonparrott I think this is ready for a second pass at review.

tswast · 2018-02-08T19:05:44Z

Note: I've added code sample snippets for all customer-managed encryption key operations in #4852, which should also serve well as system tests. (The tests all pass)

bigquery/google/cloud/bigquery/job.py

+
+    @destination_encryption_configuration.setter
+    def destination_encryption_configuration(self, value):
+        self._properties[


bigquery/google/cloud/bigquery/table.py

+
+    def __init__(self, **kwargs):
+        self._properties = {}
+        if 'kms_key_name' in kwargs:


theacodes · 2018-02-08T22:42:10Z

lgtm once you appease the ci

Add support for EncryptionConfiguration

c1b4c0a

vitaliili requested a review from lukesneeringer as a code owner February 6, 2018 22:23

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 6, 2018

theacodes requested a review from tswast February 6, 2018 22:53

vitaliili added 3 commits February 6, 2018 15:14

Add support for EncryptionConfiguration

685b013

Add support for EncryptionConfiguration

1f72102

Add support for EncryptionConfiguration

abb1995

tswast requested changes Feb 7, 2018

View reviewed changes

vitaliili added 8 commits February 6, 2018 17:16

Add support for EncryptionConfiguration

1970146

Add support for EncryptionConfiguration

5aac241

Add support for EncryptionConfiguration

68d7ee8

Add support for EncryptionConfiguration

684c558

Add support for EncryptionConfiguration

d2d5e01

Add support for EncryptionConfiguration

d2c927b

Add support for EncryptionConfiguration

fb955e6

Add support for EncryptionConfiguration

35f7646

tswast added 2 commits February 7, 2018 14:08

Fix docstrings.

050ec20

Remove unnecessary docstring from kms_key_name setter.

333879c

tswast approved these changes Feb 7, 2018

View reviewed changes

theacodes reviewed Feb 7, 2018

View reviewed changes

tswast mentioned this pull request Feb 7, 2018

BigQuery: add code snippet for CMEK table loading. #4852

Merged

tswast requested changes Feb 7, 2018

View reviewed changes

vitaliili and others added 7 commits February 7, 2018 18:21

Add support for EncryptionConfiguration

6990a17

Add support for EncryptionConfiguration

c3d55cd

Use Napoleon-style for EncryptionConfiguration docstrings.

89c7d25

Include EncryptionConfiguration in BigQuery module. Use _properties for EncryptionConfiguration implementation.

Use Napoleon-style for destination_encryption_configuration docstrings.

7257083

Fix EncryptionConfiguration kms_key_name getter.

44ad9b0

Note: I use _properties and kwargs because we want to be able to explicitly set kmsKeyName to None to allow unsetting it.

Fix unit tests for EncryptionConfiguration.

97c006f

Also, fix links to job configs for destination_encryption_configurations.

Add setter for kms_key_name.

0fb945a

tswast approved these changes Feb 8, 2018

View reviewed changes

theacodes suggested changes Feb 8, 2018

View reviewed changes

Allow setting encryption configuration to None.

47f0b1d

theacodes approved these changes Feb 8, 2018

View reviewed changes

Fix lint errors in job unit tests.

ac0cd1f

tswast merged commit 438f4a3 into googleapis:master Feb 8, 2018

Add support for EncryptionConfiguration #4845

Add support for EncryptionConfiguration #4845

Uh oh!

Conversation

vitaliili commented Feb 6, 2018

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

tswast commented Feb 7, 2018

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

tswast left a comment

Choose a reason for hiding this comment

Uh oh!

tswast left a comment

Choose a reason for hiding this comment

Uh oh!

tswast commented Feb 8, 2018

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!