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

Batched Requests don't honor the host setting #3254

Closed
albertzaharovits opened this issue May 11, 2018 · 4 comments · Fixed by #3340
Closed

Batched Requests don't honor the host setting #3254

albertzaharovits opened this issue May 11, 2018 · 4 comments · Fixed by #3340
Assignees
Labels
api: storage Issues related to the Cloud Storage API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@albertzaharovits
Copy link

Batch endpoint always hits www.googleapis.com irrespective of the host setting on the StorageOptions that built the storage client.
https://github.com/GoogleCloudPlatform/google-cloud-java/blob/52b727aef88ae76984aa3c02b4d7067e198d34b7/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java#L191

@albertzaharovits albertzaharovits changed the title Batched Requests don't StorageOptions#getHost() Batched Requests don't honor the host setting May 11, 2018
@yihanzhen
Copy link
Contributor

Hi @albertzaharovits ,
IIRC batch requests of all google apis have to hit "https://www.googleapis.com/batch/[api_name]/[endpoint]". Is there a reason you want to change the batch URL?

@yihanzhen yihanzhen added type: question Request for information or clarification. Not an issue. api: storage Issues related to the Cloud Storage API. labels May 18, 2018
@yihanzhen yihanzhen self-assigned this May 18, 2018
@pongad
Copy link
Contributor

pongad commented May 21, 2018

@hzyi-google I think we should fix this. For example, the user could be pointing the client to a test instance when integration testing. Right now, everything will hit the test instance, but batches will get to the real one, which is probably really confusing.

@albertzaharovits
Copy link
Author

Hi @hzyi-google ,

I was expecting that StorageOptionsBuilder.setHost be consistent across APIs. It is impractical if only certain APIs follow this setting.

As @pongad hinted, our usecase is in the scope of integration testing. Specifically the GCS repository plugin for elasticsearch. The mock integ server is here: https://github.com/elastic/elasticsearch/blob/master/plugins/repository-gcs/qa/google-cloud-storage/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageTestServer.java

We have skirted around it by using this hack: https://github.com/elastic/elasticsearch/blob/c351b51ac4f322ec5453806a8a0af027a71d5802/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageService.java#L121

@yihanzhen
Copy link
Contributor

@pongad @albertzaharovits Thanks for pointing this out. I'll work on this.

@yihanzhen yihanzhen added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed type: question Request for information or clarification. Not an issue. labels May 30, 2018
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. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants