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
[Feature Request: Cloud Spanner] Allow unnecessary parameters to be nil when using Cloud Spanner Emulator #8134
Comments
@aeroastro thanks for reporting this issue. I didn't implement the Ruby version but from my understanding of how it works in the client libraries for other languages, we're meant to set the We want the emulator usage to be consistent across all the Spanner client libraries (and across products) which is why it was implemented this way. Does the environment variable not work in the setup you have? Or do you prefer not to use that option? I'd like to understand the requirements before deciding the next steps. |
Sorry for the late reply. current code does not work intuitivelyFollowing code generates an exception even when the environment variable is set. ENV['SPANNER_EMULATOR_HOST'] = 'localhost:9010' # This is the same as `export SPANNER_EMULATOR_HOST=9010`
require 'google/cloud/spanner'
spanner = Google::Cloud::Spanner.new The exception is as follows:
This means that the environment variables are implicitly coupled in the library and the objective of the doc is not fully achieved. So, the unnecessary project and project credentials problem exists both in argument option way and in environment variable way. prefer to use the argument and the limitation of environment variablesFirst, it is pretty common for Ruby on Rails to define the database definition explicitly in the definition file, and the https://guides.rubyonrails.org/active_record_multiple_databases.html#setting-up-your-application Especially, I would like to keep the explicit database configuration even with the development:
adapter: "spanner"
emulator_host: localhost:9010
project: "emulator-project" # This line can be removed by the conceptual design above
instance: "emulator-instance" # This line can be removed by the conceptual design above
database: "app-dev"
production:
adapter: "spanner"
project: "<google project name>"
instance: "<google instance name>"
credentials: "<google credentails file path>"
database: "app-prod" Also, in some environment, argument style is more suitable.
In the conceptual design, what I would like to do is to achieve the same objective on the argument interface.
|
Thanks for the explanation @aeroastro. I see what you mean now. @jiren would you be able to take the @aeroastro's design and see if it can be supported in the Ruby client? |
@aeroastro only credentials we should move inside the Still need to check with the proxy set using emulator host required credentials or not. |
@jiren Is it OK now to move on to making a Pull Request? |
Yes please create a pull request. |
Is your feature request related to a problem? Please describe.
It is often for me to use Cloud Spanner Emulator to develop app backended by Cloud Spanner.
However, current I/F of
Google::Cloud::Spanner.new
raises error when seemingly-unnecessary parameter is missing.Following are the reproduction code.
This raises the following error.
Also, after running the following code with
credentials
argument,it still raises error.
I think the above parameters does not hold strong meaning when using emulator.
(If the above assumption is not correct, could you tell me about it?)
Describe the solution you'd like
I want the following code to be validly executed.
I have written the conceptual design as follows.
https://github.com/googleapis/google-cloud-ruby/compare/master...aeroastro:feature/reduce-required-arguments-for-spanner-emulator?expand=1
When using this code, following code works, and we can easily understand that it connects to emulator.
Describe alternatives you've considered
When we specify the arguments as follows, it does not raise exception.
However, I would like to omit the seemingly unnecessary arguments, especially for
credentials
, which is overwritten by:this_channel_is_insecure
in the current code.Additional context
Also, an incompatible change (which does not affect production environments) is included in the conceptual design to reduce the internal complexity of
Google::Cloud::Spanner.new
.I would like to hear some opinion on the change. If required, I can add code to maintain backward compatibility.
The text was updated successfully, but these errors were encountered: