Skip to content

chore: refactor GrpcStorageImpl#read* methods to use more specific channels#1474

Merged
BenWhitehead merged 2 commits intofeat/grpc-storagefrom
smarter-read
Jun 28, 2022
Merged

chore: refactor GrpcStorageImpl#read* methods to use more specific channels#1474
BenWhitehead merged 2 commits intofeat/grpc-storagefrom
smarter-read

Conversation

@BenWhitehead
Copy link
Collaborator

Update readAllBytes and downloadTo methods to use unbuffered read channel, instead of reader

Update *ReadableByteChannelSessions to take a ReadObjectRequest instead of Object

Update Hasher#validate to be lazy with regard to the ByteBuffer, this ensures no new byte buffer is created if it isn't needed in the case of noop.

Add StorageArbitraries.Objects#name()
Move StorageArbitraries.bucketName() -> StorageArbitraries.Buckets#name()
Add StorageArbitraries.alnum() and cleanup usages in Buckets.name()

…annels

Update readAllBytes and downloadTo methods to use unbuffered read channel, instead of reader

Update *ReadableByteChannelSessions to take a ReadObjectRequest instead of Object

Update Hasher#validate to be lazy with regard to the ByteBuffer, this ensures no new byte buffer is created if it isn't needed in the case of noop.

Add StorageArbitraries.Objects#name()
Move StorageArbitraries.bucketName() -> StorageArbitraries.Buckets#name()
Add StorageArbitraries.alnum() and cleanup usages in Buckets.name()
@BenWhitehead BenWhitehead requested a review from a team June 27, 2022 18:59
@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 Jun 27, 2022
String userProject = (String) optionsMap.get(StorageRpc.Option.USER_PROJECT);
if (userProject != null) {
// TODO: user project
// ReadObjectRequest does not have a user_project field
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, we need to use gRPC Metadata for this now per cl/448634966

I see: https://g3doc.corp.google.com/net/grpc/g3doc/java/life_of_a_stream_java.md?cl=head#clientinterceptor

Looks like we need to use a client side interceptor but couldn't figure out where to attach it. Maybe you'll have better luck.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added an item to the tracker to implement this in a separate PR


final Map<StorageRpc.Option, ?> optionsMap = StorageImpl.optionMap(options);

// TODO: handle StorageRpc.Option.RETURN_RAW_INPUT_STREAM impacts
Copy link
Contributor

Choose a reason for hiding this comment

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

gRPC will always provide compressed data; maybe we can decompress data for the user to match up with the existing JSON API experience

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added an item to the tracker to implement this in a separate PR

@BenWhitehead BenWhitehead merged commit 61186d7 into feat/grpc-storage Jun 28, 2022
@BenWhitehead BenWhitehead deleted the smarter-read branch June 28, 2022 15:59
gcf-owl-bot bot added a commit that referenced this pull request Jun 29, 2022
Source-Link: googleapis/synthtool@7a220e2
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:6d4e3a15c62cfdcb823d60e16da7521e7c6fc00eba07c8ff12e4de9924a57d28
gcf-merge-on-green bot pushed a commit that referenced this pull request Jun 30, 2022
Source-Link: googleapis/synthtool@7a220e2
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:6d4e3a15c62cfdcb823d60e16da7521e7c6fc00eba07c8ff12e4de9924a57d28
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. size: l Pull request size is large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants