Skip to content
This repository was archived by the owner on Dec 8, 2021. It is now read-only.

feat: make emulator endpoint/credentials changes as late as possible #1277

Merged
merged 1 commit into from
Feb 21, 2020

Conversation

devbww
Copy link
Contributor

@devbww devbww commented Feb 21, 2020

Rather than making the ${SPANNER_EMULATOR_HOST} endpoint/credentials
changes during ConnectionOptions construction (which then requires
that we ignore further attempts to set those fields), do them as late
as possible (i.e., just before creating each variety of channel).

This then facilitates the use of a common ConnectionOptions class
between Cloud C++ projects by not requiring special behavior in the
set functions.

While this modifies the signatures of CreateDefault*Stub(), it is
only a change from pass-by-const-ref to pass-by-value, which will not
require any change to user code.


This change is Reviewable

Rather than making the `${SPANNER_EMULATOR_HOST}` endpoint/credentials
changes during `ConnectionOptions` construction (which then requires
that we ignore further attempts to set those fields), do them as late
as possible (i.e., just before creating each variety of channel).

This then facilitates the use of a common `ConnectionOptions` class
between Cloud C++ projects by not requiring special behavior in the
`set` functions.

While this modifies the signatures of `CreateDefault*Stub()`, it is
only a change from pass-by-const-ref to pass-by-value, which will not
require any change to user code.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 21, 2020
@codecov
Copy link

codecov bot commented Feb 21, 2020

Codecov Report

Merging #1277 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1277      +/-   ##
==========================================
+ Coverage   94.61%   94.61%   +<.01%     
==========================================
  Files         178      180       +2     
  Lines       13830    13828       -2     
==========================================
- Hits        13085    13084       -1     
+ Misses        745      744       -1
Impacted Files Coverage Δ
...oogle/cloud/spanner/internal/instance_admin_stub.h 100% <ø> (ø)
google/cloud/spanner/internal/spanner_stub.h 100% <ø> (ø) ⬆️
google/cloud/spanner/connection_options_test.cc 100% <100%> (ø) ⬆️
google/cloud/spanner/internal/spanner_stub.cc 64.64% <100%> (-1.69%) ⬇️
google/cloud/spanner/connection_options.cc 97.67% <100%> (+0.05%) ⬆️
...ogle/cloud/spanner/internal/database_admin_stub.cc 83.33% <100%> (+0.21%) ⬆️
google/cloud/spanner/connection_options.h 100% <100%> (ø) ⬆️
...ogle/cloud/spanner/internal/instance_admin_stub.cc 83.33% <100%> (+0.2%) ⬆️
google/cloud/spanner/internal/log_wrapper.h 71.42% <0%> (-13.19%) ⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20e2f9b...e8a6095. Read the comment docs.

@devbww devbww marked this pull request as ready for review February 21, 2020 09:04
@devbww devbww requested a review from devjgm February 21, 2020 09:04
@devbww devbww merged commit b1bf2e6 into googleapis:master Feb 21, 2020
@devbww devbww deleted the emulator-common-connection-options branch February 21, 2020 16:03
devjgm pushed a commit to devjgm/google-cloud-cpp that referenced this pull request May 7, 2020
…oogleapis/google-cloud-cpp-spanner#1277)

Rather than making the `${SPANNER_EMULATOR_HOST}` endpoint/credentials
changes during `ConnectionOptions` construction (which then requires
that we ignore further attempts to set those fields), do them as late
as possible (i.e., just before creating each variety of channel).

This then facilitates the use of a common `ConnectionOptions` class
between Cloud C++ projects by not requiring special behavior in the
`set` functions.

While this modifies the signatures of `CreateDefault*Stub()`, it is
only a change from pass-by-const-ref to pass-by-value, which will not
require any change to user code.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants