Skip to content

Conversation

@BenWhitehead
Copy link
Collaborator

Publicly exposing this is gated on how/when we are able to expose StorageReadChannel.

@BenWhitehead BenWhitehead added the owlbot:ignore instruct owl-bot to ignore a PR label Jan 11, 2023
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: storage Issues related to the googleapis/java-storage API. labels Jan 11, 2023
Base automatically changed from grpc/read-channel/seek-after-read to main January 12, 2023 17:40
@BenWhitehead BenWhitehead force-pushed the read/expose-shallow-metadata branch from 1c9d74d to 6eddc2c Compare January 17, 2023 18:53
@BenWhitehead BenWhitehead marked this pull request as ready for review January 17, 2023 18:55
@BenWhitehead BenWhitehead requested a review from a team as a code owner January 17, 2023 18:55
…ncluded in a read media response for both json and grpc

Publicly exposing this is gated on how/when we are able to expose
StorageReadChannel.
@BenWhitehead BenWhitehead force-pushed the read/expose-shallow-metadata branch from 6eddc2c to b9a2fd2 Compare January 17, 2023 18:57
@BenWhitehead BenWhitehead requested a review from a team January 17, 2023 18:57
if (xGoogGenHeader != null) {
StorageObject clone = apiaryReadRequest.getObject().clone();
ifNonNull(xGoogGenHeader, Long::valueOf, clone::setGeneration);
ifNonNull(xGoogGenHeader, Long::valueOf, g -> this.xGoogGeneration = g);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mentioned yesterday you were going to add a comment for this line

return ensureResponseIteratorOpen().hasNext();
} catch (RuntimeException e) {
if (!result.isDone()) {
result.setException(StorageException.coalesce(e));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we set the exception on the result only if the result is not complete? What case is this covering?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A call to hasNext()/next() could error any time. More often than not, as long as the object exists, this code will not run, but when the object doesn't exist this ensures we communicate that to the result as well as any call to read

return ensureResponseIteratorOpen().next();
} catch (RuntimeException e) {
if (!result.isDone()) {
result.setException(StorageException.coalesce(e));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question as above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same answer as above

@BenWhitehead BenWhitehead force-pushed the read/expose-shallow-metadata branch from b0e0d26 to 0a4a6db Compare January 18, 2023 23:39
@BenWhitehead BenWhitehead merged commit 2b94567 into main Jan 20, 2023
@BenWhitehead BenWhitehead deleted the read/expose-shallow-metadata branch January 20, 2023 19:00
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/java-storage API. owlbot:ignore instruct owl-bot to ignore a PR size: l Pull request size is large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants