Skip to content

fix(storage): preserve supplied endpoint's scheme - #4609

Merged
tritone merged 7 commits into
googleapis:masterfrom
BrennaEpp:preserve-endpoint-scheme
Aug 13, 2021
Merged

tritone merged 7 commits into
googleapis:masterfrom
BrennaEpp:preserve-endpoint-scheme

Conversation

@BrennaEpp

Copy link
Copy Markdown
Contributor

Fixes #3509

option.WithEndpoint() completely overrides STORAGE_EMULATOR_HOST environment variable.

Built on #4608

@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Aug 13, 2021
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Aug 13, 2021
@BrennaEpp
BrennaEpp marked this pull request as ready for review August 13, 2021 17:16
@BrennaEpp
BrennaEpp requested review from a team and tritone August 13, 2021 17:16

@tritone tritone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

New test cases look good.

Comment thread storage/storage.go Outdated
return nil, fmt.Errorf("supplied endpoint %q is not valid: %v", ep, err)
}
readHost = u.Host
scheme = u.Scheme

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like we set scheme in L114 & L123 above-- I guess this no longer does anything?

Also, is it possible for the returned endpoint from htransport.NewClient to have an empty scheme?

I'm thinking we should keep L114 & L123 as is and only set scheme here if u.Scheme != "", just out of caution. Does that make sense?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The endpoint for htransport.NewClient should always have a scheme as long as we send in the WithDefaultEndpoint option. But yes that makes sense, since it does not actually promise to.

@tritone
tritone merged commit ee2756f into googleapis:master Aug 13, 2021
gcf-merge-on-green Bot pushed a commit that referenced this pull request Aug 31, 2021
🤖 I have created a release \*beep\* \*boop\*
---
### [1.16.1](https://www.github.com/googleapis/google-cloud-go/compare/storage/v1.16.0...storage/v1.16.1) (2021-08-30)


### Bug Fixes

* **storage/internal:** Update encryption_key fields to "bytes" type. fix: Improve date/times and field name clarity in lifecycle conditions. ([a52baa4](https://www.github.com/googleapis/google-cloud-go/commit/a52baa456ed8513ec492c4b573c191eb61468758))
* **storage:** accept emulator env var without scheme ([#4616](https://www.github.com/googleapis/google-cloud-go/issues/4616)) ([5f8cbb9](https://www.github.com/googleapis/google-cloud-go/commit/5f8cbb98070109e2a34409ac775ed63b94d37efd))
* **storage:** preserve supplied endpoint's scheme ([#4609](https://www.github.com/googleapis/google-cloud-go/issues/4609)) ([ee2756f](https://www.github.com/googleapis/google-cloud-go/commit/ee2756fb0a335d591464a770c9fa4f8fe0ba2e01))
* **storage:** remove unnecessary variable ([#4608](https://www.github.com/googleapis/google-cloud-go/issues/4608)) ([27fc784](https://www.github.com/googleapis/google-cloud-go/commit/27fc78456fb251652bdf5cdb493734a7e1e643e1))
* **storage:** retry LockRetentionPolicy ([#4439](https://www.github.com/googleapis/google-cloud-go/issues/4439)) ([09879ea](https://www.github.com/googleapis/google-cloud-go/commit/09879ea80cb67f9bfd8fc9384b0fda335567cba9)), refs [#4437](https://www.github.com/googleapis/google-cloud-go/issues/4437)
* **storage:** revise Reader to send XML preconditions ([#4479](https://www.github.com/googleapis/google-cloud-go/issues/4479)) ([e36b29a](https://www.github.com/googleapis/google-cloud-go/commit/e36b29a3d43bce5c1c044f7daf6e1db00b0a49e0)), refs [#4470](https://www.github.com/googleapis/google-cloud-go/issues/4470)

This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@BrennaEpp
BrennaEpp deleted the preserve-endpoint-scheme branch March 23, 2022 19:07
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. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants