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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

fix: revise blob.compose query parameters if_generation_match #454

Merged
merged 16 commits into from Jun 14, 2021
Merged

fix: revise blob.compose query parameters if_generation_match #454

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
a1a66c3
revise blob.compose logic to match API usage
cojenco Jun 4, 2021
bd7407a
update tests
cojenco Jun 5, 2021
a896eff
update system test
cojenco Jun 5, 2021
5dec505
Merge branch 'master' into obj-compose-retry
cojenco Jun 7, 2021
b4363af
address comments
cojenco Jun 7, 2021
87a7411
Merge branch 'master' into obj-compose-retry
tseaver Jun 8, 2021
13e221f
馃 Updates from OwlBot
gcf-owl-bot[bot] Jun 8, 2021
1c1d70d
Merge branch 'master' into obj-compose-retry
tseaver Jun 8, 2021
42be461
revise logic for backwards compatibility
cojenco Jun 8, 2021
b94f05c
add tests
cojenco Jun 9, 2021
b6ce5ef
revise docstring
cojenco Jun 10, 2021
8526054
Merge main branch
cojenco Jun 10, 2021
3958562
fix test
cojenco Jun 10, 2021
941da79
revise to DeprecationWarning
cojenco Jun 11, 2021
23be964
address comments and revise docstrings
cojenco Jun 11, 2021
e2c051c
Merge branch 'master' into obj-compose-retry
cojenco Jun 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 31 additions & 33 deletions google/cloud/storage/blob.py
View file
Open in desktop
Expand Up @@ -3055,6 +3055,7 @@ def compose(
timeout=_DEFAULT_TIMEOUT,
if_generation_match=None,
if_metageneration_match=None,
if_source_generation_match=None,
retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED,
):
"""Concatenate source blobs into this one.
Expand All @@ -3077,19 +3078,24 @@ def compose(
Can also be passed as a tuple (connect_timeout, read_timeout).
See :meth:`requests.Session.request` documentation for details.

:type if_generation_match: list of long
:type if_generation_match: long
:param if_generation_match:
(Optional) Make the operation conditional on whether the blob's
current generation matches the given value. Setting to 0 makes the
operation succeed only if there are no live versions of the blob.
The list must match ``sources`` item-to-item.

:type if_metageneration_match: list of long
:param if_metageneration_match:
(Optional) Make the operation conditional on whether the blob's
current metageneration matches the given value. The list must match
``sources`` item-to-item.

:type if_source_generation_match: list of long
:param if_source_generation_match:
(Optional) Make the operation conditional on whether the current generation
of each source blob matches the corresponding generation.
The list must match ``sources`` item-to-item.

:type retry: google.api_core.retry.Retry or google.cloud.storage.retry.ConditionalRetryPolicy
:param retry: (Optional) How to retry the RPC. A None value will disable retries.
A google.api_core.retry.Retry value will enable retries, and the object will
Expand All @@ -3099,61 +3105,53 @@ def compose(
activates it only if certain conditions are met. This class exists to provide safe defaults
for RPC calls that are not technically safe to retry normally (due to potential data
duplication or other side-effects) but become safe to retry if a condition such as
if_metageneration_match is set.
if_generation_match is set.

See the retry.py source code and docstrings in this package (google.cloud.storage.retry) for
information on retry types and how to configure them.

Example:
Compose blobs using generation match preconditions.
Compose blobs using source generation match preconditions.

>>> from google.cloud import storage
>>> client = storage.Client()
>>> bucket = client.bucket("bucket-name")

>>> blobs = [bucket.blob("blob-name-1"), bucket.blob("blob-name-2")]
>>> if_generation_match = [None] * len(blobs)
>>> if_generation_match[0] = "123" # precondition for "blob-name-1"
>>> if_source_generation_match = [None] * len(blobs)
>>> if_source_generation_match[0] = "123" # precondition for "blob-name-1"

>>> composed_blob = bucket.blob("composed-name")
>>> composed_blob.compose(blobs, if_generation_match)
>>> composed_blob.compose(blobs, if_source_generation_match=if_source_generation_match)
"""
sources_len = len(sources)
tseaver marked this conversation as resolved.
Show resolved Hide resolved
if if_generation_match is not None and len(if_generation_match) != sources_len:
raise ValueError(
"'if_generation_match' length must be the same as 'sources' length"
)

if (
if_metageneration_match is not None
and len(if_metageneration_match) != sources_len
):
raise ValueError(
"'if_metageneration_match' length must be the same as 'sources' length"
)

client = self._require_client(client)
query_params = {}

if self.user_project is not None:
query_params["userProject"] = self.user_project

_add_generation_match_parameters(
query_params,
if_generation_match=if_generation_match,
if_metageneration_match=if_metageneration_match,
)

tseaver marked this conversation as resolved.
Show resolved Hide resolved
if if_source_generation_match is None:
if_source_generation_match = [None] * sources_len
if len(if_source_generation_match) != sources_len:
raise ValueError(
"'if_source_generation_match' length must be the same as 'sources' length"
)

source_objects = []
for index, source in enumerate(sources):
source_object = {"name": source.name}
for source, source_generation in zip(sources, if_source_generation_match):
source_object = {"name": source.name, "generation": source.generation}

preconditions = {}
if (
if_generation_match is not None
and if_generation_match[index] is not None
):
preconditions["ifGenerationMatch"] = if_generation_match[index]

if (
if_metageneration_match is not None
and if_metageneration_match[index] is not None
):
preconditions["ifMetagenerationMatch"] = if_metageneration_match[index]
if source_generation is not None:
preconditions["ifGenerationMatch"] = source_generation

if preconditions:
source_object["objectPreconditions"] = preconditions
Expand Down Expand Up @@ -3274,7 +3272,7 @@ def rewrite(
activates it only if certain conditions are met. This class exists to provide safe defaults
for RPC calls that are not technically safe to retry normally (due to potential data
duplication or other side-effects) but become safe to retry if a condition such as
if_metageneration_match is set.
if_generation_match is set.

See the retry.py source code and docstrings in this package (google.cloud.storage.retry) for
information on retry types and how to configure them.
Expand Down
31 changes: 23 additions & 8 deletions tests/system/test_system.py
View file
Open in desktop
Expand Up @@ -1723,7 +1723,7 @@ def test_compose_replace_existing_blob(self):
composed = original.download_as_bytes()
self.assertEqual(composed, BEFORE + TO_APPEND)

def test_compose_with_generation_match(self):
def test_compose_with_source_generation_match(self):
BEFORE = b"AAA\n"
original = self.bucket.blob("original")
original.content_type = "text/plain"
Expand All @@ -1736,21 +1736,36 @@ def test_compose_with_generation_match(self):
self.case_blobs_to_delete.append(to_append)

with self.assertRaises(google.api_core.exceptions.PreconditionFailed):
original.compose(
[original, to_append],
if_generation_match=[6, 7],
if_metageneration_match=[8, 9],
)
original.compose([original, to_append], if_source_generation_match=[6, 7])

original.compose(
[original, to_append],
if_generation_match=[original.generation, to_append.generation],
if_metageneration_match=[original.metageneration, to_append.metageneration],
if_source_generation_match=[original.generation, to_append.generation],
)

composed = original.download_as_bytes()
self.assertEqual(composed, BEFORE + TO_APPEND)
cojenco marked this conversation as resolved.
Show resolved Hide resolved

def test_compose_with_generation_match(self):
BEFORE = b"AAA\n"
original = self.bucket.blob("original")
original.content_type = "text/plain"
original.upload_from_string(BEFORE)
self.case_blobs_to_delete.append(original)

TO_APPEND = b"BBB\n"
to_append = self.bucket.blob("to_append")
to_append.upload_from_string(TO_APPEND)
self.case_blobs_to_delete.append(to_append)

with self.assertRaises(google.api_core.exceptions.PreconditionFailed):
original.compose([original, to_append], if_generation_match=0)

original.compose([original, to_append], if_generation_match=original.generation)

composed = original.download_as_bytes()
self.assertEqual(composed, BEFORE + TO_APPEND)

@unittest.skipUnless(USER_PROJECT, "USER_PROJECT not set in environment.")
def test_compose_with_user_project(self):
new_bucket_name = "compose-user-project" + unique_resource_id("-")
Expand Down
90 changes: 65 additions & 25 deletions tests/unit/test_blob.py
View file
Open in desktop
Expand Up @@ -3549,7 +3549,10 @@ def test_compose_wo_content_type_set(self):
"path": "/b/name/o/%s/compose" % DESTINATION,
"query_params": {},
"data": {
"sourceObjects": [{"name": source_1.name}, {"name": source_2.name}],
"sourceObjects": [
{"name": source_1.name, "generation": source_1.generation},
{"name": source_2.name, "generation": source_2.generation},
],
"destination": {},
},
"_target_object": destination,
Expand Down Expand Up @@ -3586,7 +3589,10 @@ def test_compose_minimal_w_user_project(self):
"path": "/b/name/o/%s/compose" % DESTINATION,
"query_params": {"userProject": USER_PROJECT},
"data": {
"sourceObjects": [{"name": source_1.name}, {"name": source_2.name}],
"sourceObjects": [
{"name": source_1.name, "generation": source_1.generation},
{"name": source_2.name, "generation": source_2.generation},
],
"destination": {"contentType": "text/plain"},
},
"_target_object": destination,
Expand Down Expand Up @@ -3624,7 +3630,10 @@ def test_compose_w_additional_property_changes(self):
"path": "/b/name/o/%s/compose" % DESTINATION,
"query_params": {},
"data": {
"sourceObjects": [{"name": source_1.name}, {"name": source_2.name}],
"sourceObjects": [
{"name": source_1.name, "generation": source_1.generation},
{"name": source_2.name, "generation": source_2.generation},
],
"destination": {
"contentType": "text/plain",
"contentLanguage": "en-US",
Expand All @@ -3637,13 +3646,12 @@ def test_compose_w_additional_property_changes(self):
},
)

def test_compose_w_generation_match(self):
def test_compose_w_source_generation_match(self):
SOURCE_1 = "source-1"
SOURCE_2 = "source-2"
DESTINATION = "destination"
RESOURCE = {}
GENERATION_NUMBERS = [6, 9]
METAGENERATION_NUMBERS = [7, 1]
SOURCE_GENERATION_NUMBERS = [6, 9]

after = ({"status": http_client.OK}, RESOURCE)
connection = _Connection(after)
Expand All @@ -3655,8 +3663,7 @@ def test_compose_w_generation_match(self):
destination = self._make_one(DESTINATION, bucket=bucket)
destination.compose(
sources=[source_1, source_2],
if_generation_match=GENERATION_NUMBERS,
if_metageneration_match=METAGENERATION_NUMBERS,
if_source_generation_match=SOURCE_GENERATION_NUMBERS,
)

kw = connection._requested
Expand All @@ -3671,16 +3678,16 @@ def test_compose_w_generation_match(self):
"sourceObjects": [
{
"name": source_1.name,
"generation": source_1.generation,
"objectPreconditions": {
"ifGenerationMatch": GENERATION_NUMBERS[0],
"ifMetagenerationMatch": METAGENERATION_NUMBERS[0],
"ifGenerationMatch": SOURCE_GENERATION_NUMBERS[0],
},
},
{
"name": source_2.name,
"generation": source_2.generation,
"objectPreconditions": {
"ifGenerationMatch": GENERATION_NUMBERS[1],
"ifMetagenerationMatch": METAGENERATION_NUMBERS[1],
"ifGenerationMatch": SOURCE_GENERATION_NUMBERS[1]
},
},
],
Expand All @@ -3692,12 +3699,11 @@ def test_compose_w_generation_match(self):
},
)

def test_compose_w_generation_match_bad_length(self):
def test_compose_w_source_generation_match_bad_length(self):
SOURCE_1 = "source-1"
SOURCE_2 = "source-2"
DESTINATION = "destination"
GENERATION_NUMBERS = [6]
METAGENERATION_NUMBERS = [7]
SOURCE_GENERATION_NUMBERS = [6]

after = ({"status": http_client.OK}, {})
connection = _Connection(after)
Expand All @@ -3708,21 +3714,17 @@ def test_compose_w_generation_match_bad_length(self):

destination = self._make_one(DESTINATION, bucket=bucket)

with self.assertRaises(ValueError):
destination.compose(
sources=[source_1, source_2], if_generation_match=GENERATION_NUMBERS
)
with self.assertRaises(ValueError):
destination.compose(
sources=[source_1, source_2],
if_metageneration_match=METAGENERATION_NUMBERS,
if_source_generation_match=SOURCE_GENERATION_NUMBERS,
)

def test_compose_w_generation_match_nones(self):
def test_compose_w_source_generation_match_nones(self):
SOURCE_1 = "source-1"
SOURCE_2 = "source-2"
DESTINATION = "destination"
GENERATION_NUMBERS = [6, None]
SOURCE_GENERATION_NUMBERS = [6, None]

after = ({"status": http_client.OK}, {})
connection = _Connection(after)
Expand All @@ -3733,7 +3735,8 @@ def test_compose_w_generation_match_nones(self):

destination = self._make_one(DESTINATION, bucket=bucket)
destination.compose(
sources=[source_1, source_2], if_generation_match=GENERATION_NUMBERS
sources=[source_1, source_2],
if_source_generation_match=SOURCE_GENERATION_NUMBERS,
)

kw = connection._requested
Expand All @@ -3748,11 +3751,48 @@ def test_compose_w_generation_match_nones(self):
"sourceObjects": [
{
"name": source_1.name,
"generation": source_1.generation,
"objectPreconditions": {
"ifGenerationMatch": GENERATION_NUMBERS[0]
"ifGenerationMatch": SOURCE_GENERATION_NUMBERS[0]
},
},
{"name": source_2.name},
{"name": source_2.name, "generation": source_2.generation},
],
"destination": {},
},
"_target_object": destination,
"timeout": self._get_default_timeout(),
"retry": DEFAULT_RETRY_IF_GENERATION_SPECIFIED,
},
)

def test_compose_w_generation_match(self):
SOURCE_1 = "source-1"
SOURCE_2 = "source-2"
DESTINATION = "destination"

after = ({"status": http_client.OK}, {})
connection = _Connection(after)
client = _Client(connection)
bucket = _Bucket(client=client)
source_1 = self._make_one(SOURCE_1, bucket=bucket)
source_2 = self._make_one(SOURCE_2, bucket=bucket)

destination = self._make_one(DESTINATION, bucket=bucket)
destination.compose(sources=[source_1, source_2], if_generation_match=1)

kw = connection._requested
self.assertEqual(len(kw), 1)
self.assertEqual(
kw[0],
{
"method": "POST",
"path": "/b/name/o/%s/compose" % DESTINATION,
"query_params": {"ifGenerationMatch": 1},
"data": {
"sourceObjects": [
{"name": source_1.name, "generation": source_1.generation},
{"name": source_2.name, "generation": source_2.generation},
],
"destination": {},
},
Expand Down