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

Batch requests generating multiple response codes for each request #1112

Closed
BjornPrime opened this issue Aug 28, 2023 · 5 comments
Closed

Batch requests generating multiple response codes for each request #1112

BjornPrime opened this issue Aug 28, 2023 · 5 comments
Assignees
Labels
api: storage Issues related to the googleapis/python-storage API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: question Request for information or clarification. Not an issue.

Comments

@BjornPrime
Copy link

Batch requests (using Batch._responses to access the response data) are returning multiple responses for each request. 4/request for copy requests, 2/request for deletes. Not sure if other methods are affected.

When raise_exception=False, this results in exceptions not being associated with the right request, or not being registered at all.

Refer to example here.

Basically, the first line of text in the quote block is printing length of requests and then length of responses. The next five lines are printing every response code associated with a particular item (four for each). The final five lines show the results as read by our implementation, notably missing the 404s thrown by the fourth item.

Obviously, we can fix our particular implementation to account for this, but thought y'all might want to know that something seems to be up with the batch responses, especially as this line seems to indicate an expectation that each request only have one response.

@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/python-storage API. label Aug 28, 2023
@cojenco cojenco self-assigned this Aug 31, 2023
@cojenco cojenco added type: question Request for information or clarification. Not an issue. priority: p3 Desirable enhancement or fix. May not be included in next release. labels Aug 31, 2023
@cojenco
Copy link
Contributor

cojenco commented Aug 31, 2023

Hi @BjornPrime, I took a look at the linked example. IIUC, the reason you are seeing 4/request for copy requests is because there are multiple operation calls being made in the batch context - get_bucket() get_blob() in addition to copy_blob()

Likewise for the delete example, replace get_bucket() API call with simply instantiating the bucket object via client.bucket(bucket_name)

Consider changing your code block to something along the lines of:

with current_batch:
    for pair in current_pairs:
        src_bucket_name, src_blob_name = parse_gcs_path(pair[0])
        dest_bucket_name, dest_blob_name = parse_gcs_path(pair[1])

        # Instantiate a bucket object instead of making a GET call
        src_bucket = self.client.bucket(src_bucket_name)
        # Instantiate a blob object instead of making a GET call
        src_blob = src_bucket.blob(src_blob_name)
        # Same here
        dest_bucket = self.client.bucket(dest_bucket_name)

        # Only one API call corresponds to one subresponse
        src_bucket.copy_blob(src_blob, dest_bucket, dest_blob_name)

@BjornPrime
Copy link
Author

Will that code block replicate the behavior of the current implementation?

@cojenco
Copy link
Contributor

cojenco commented Aug 31, 2023

Yes it should; I'm assuming the objective is to copy the blob. It's important that only the copy_blob API call is wrapped in the batch context. The changes are mainly removing the extra API calls.

@BjornPrime
Copy link
Author

Okay, I think I was just concerned about making sure the right buckets and blobs were identified but it's not surprising to me if just passing the correctly named objects to the client allows it to figure that part out on its own.

@cojenco
Copy link
Contributor

cojenco commented Sep 6, 2023

Right, hope this clarifies your question. I'm closing this issue for now. Feel free to reopen if you have further questions.

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 googleapis/python-storage API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

2 participants