Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'Blob.update_storage_class' API method. #3051

Merged
merged 3 commits into from
Feb 27, 2017
Merged

Add 'Blob.update_storage_class' API method. #3051

merged 3 commits into from
Feb 27, 2017

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Feb 22, 2017

Closes #2991.

@tseaver tseaver added api: storage Issues related to the Cloud Storage API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Feb 22, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 22, 2017
@@ -12,6 +12,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# pylint: disable=too-many-lines

This comment was marked as spam.

This comment was marked as spam.

@@ -12,6 +12,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# pylint: disable=too-many-lines

This comment was marked as spam.

@@ -73,6 +75,9 @@ class Blob(_PropertyMixin):
_CHUNK_SIZE_MULTIPLE = 256 * 1024
"""Number (256 KB, in bytes) that must divide the chunk size."""

_STORAGE_CLASSES = (
'STANDARD', 'NEARLINE', 'MULTI_REGIONAL', 'REGIONAL', 'COLDLINE')

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@@ -852,6 +857,31 @@ def rewrite(self, source, token=None, client=None):

return api_response['rewriteToken'], rewritten, size

def update_storage_class(self, new_class, client=None):

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@@ -852,6 +857,31 @@ def rewrite(self, source, token=None, client=None):

return api_response['rewriteToken'], rewritten, size

def update_storage_class(self, new_class, client=None):
"""Rewrite source blob into this one.

This comment was marked as spam.

This comment was marked as spam.

:param new_class: new storage class for the object

:type client: :class:`~google.cloud.storage.client.Client` or
``NoneType``

This comment was marked as spam.

This comment was marked as spam.

self._encryption_key, source=True))

api_response = client._connection.api_request(
method='POST', path=self.path + '/rewriteTo' + self.path,

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.


api_response = client._connection.api_request(
method='POST', path=self.path + '/rewriteTo' + self.path,
data={'storageClass': new_class}, headers=headers,

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

self.assertNotIn('X-Goog-Copy-Source-Encryption-Key-Sha256', headers)
self.assertNotIn('X-Goog-Encryption-Algorithm', headers)
self.assertNotIn('X-Goog-Encryption-Key', headers)
self.assertNotIn('X-Goog-Encryption-Key-Sha256', headers)

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor

dhermes commented Feb 23, 2017

Two remaining sticking points:

  • A tracking issue for obj. storage class in the Blob constructor
  • Making sure we don't repeat ourselves on the list of storage classes in blob and bucket modules

@tseaver
Copy link
Contributor Author

tseaver commented Feb 23, 2017

@dhermes

A tracking issue for obj. storage class in the Blob constructor

#3062.

Making sure we don't repeat ourselves on the list of storage classes in blob and bucket modules

The two sets aren't identical: in particular, the DRA class is only documented for Buckets, although it is deprecated, while the 'STANDARD' class isn't documented for POSC at all (although it happens to work).

@lukesneeringer
Copy link
Contributor

lukesneeringer commented Feb 23, 2017

The two sets aren't identical: in particular, the DRA class is only documented for Buckets, although it is deprecated, while the 'STANDARD' class isn't documented for POSC at all (although it happens to work).

Is one a strict superset of the other? If so, the module with the larger set could import the smaller one and .union on it. (You'd also have to change to sets rather than tuples, which would be fine.)

If neither set is a strict superset of the other, I agree with @tseaver that they should just be separate (rather than, say, having a base set that is the intersection).

@tseaver
Copy link
Contributor Author

tseaver commented Feb 23, 2017

Is one a strict superset of the other?

@lukesneeringer By inspection of the documented values at point-of-last-reading, yes: however, the docs don't specify that: they just describe the two sets which happen to overlap:

We could perhaps coalesce the two lists if we dropped support for setting the DRA type on buckets, or if we could get the storage back-end folks to document the possible values definitively for both buckets and objects.

@lukesneeringer
Copy link
Contributor

Okay, got it. Based on that, I think I am going to say that we stick with @tseaver's approach as implemented for now (while acknowledging that it is a close call).

@dhermes
Copy link
Contributor

dhermes commented Feb 24, 2017

@lukesneeringer @tseaver We should

  • include a comment in code explaining why the duplication
  • give feedback to the backend folks that we had to duplicate / the choices don't seem clear from the docs (/cc @jonparrott)

@tseaver
Copy link
Contributor Author

tseaver commented Feb 24, 2017

@dhermes c34cd867be81cf adds docstrings to both class attributes, including links to the relevant back-end documentation.

Re-order to match canonical order in docs.

Add docstrings with links to relevant docs.

Explain why the two lists differ.
@tseaver
Copy link
Contributor Author

tseaver commented Feb 27, 2017

@dhermes, @lukesneeringer Any further issues?

@tseaver tseaver merged commit de5d8e1 into googleapis:master Feb 27, 2017
@tseaver tseaver deleted the 2991-storage-per_object_storage_class branch March 3, 2017 21:51
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
…ct_storage_class

Add 'Blob.update_storage_class' API method.
crwilcox pushed a commit to googleapis/python-storage that referenced this pull request Jan 31, 2020
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants