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

fix(storage): consistent endpoint computation #5095

Merged

Conversation

coryan
Copy link
Contributor

@coryan coryan commented Sep 19, 2020

The interaction of ClientOptions::set_endpoint() and the
CLOUD_STORAGE_TESTBENCH_ENDPOINT environment variable was inconsistent
across endpoints. For JSON endpoints set_endpoint() overrode the
CLOUD_STORAGE_TESTBENCH_ENDPOINT value, while for XML endpoints it was
the opposite.

In other libraries the environment variable always wins, so we are
changing the behavior here. This behavior was never documented, and it
was buggy, this is not a breaking change per-se, but should be
highlighted in the CHANGELOG.

I refactored the code to compute the different endpoints to standalone
functions. That makes them easier to test.

I used friend functions in internal:: over member functions in
ClientOptions because we will (some day) remove the XML endpoints and
I do not want to make XmlFoo() part of the public interface.

Fixes #5094 part of the work for #5072


This change is Reviewable

The interaction of `ClientOptions::set_endpoint()` and the
`CLOUD_STORAGE_TESTBENCH_ENDPOINT` environment variable was inconsistent
across endpoints. For JSON endpoints `set_endpoint()` overrode the
`CLOUD_STORAGE_TESTBENCH_ENDPOINT` value, while for XML endpoints it was
the opposite.

In other libraries the environment variable always wins, so we are
changing the behavior here. This behavior was never documented, and it
was buggy, this is not a breaking change per-se, but should be
highlighted in the CHANGELOG.

I refactored the code to compute the different endpoints to standalone
functions. That makes them easier to test.

I used friend functions in `internal::` over member functions in
`ClientOptions` because we will (some day) remove the XML endpoints and
I do not want to make `XmlFoo()` part of the public interface.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 19, 2020
@coryan coryan marked this pull request as ready for review September 19, 2020 00:52
@coryan coryan requested a review from a team as a code owner September 19, 2020 00:52
CHANGELOG.md Show resolved Hide resolved
@coryan coryan merged commit abc8e9c into googleapis:master Sep 19, 2020
@coryan coryan deleted the fix-storage-consistent-endpoint-computation branch September 19, 2020 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent computation of XML vs. JSON endpoints
3 participants