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

Jump to bottom

feat: pass 'client_options' to base class ctor #60

Merged
merged 2 commits into from Aug 5, 2020
Merged

feat: pass 'client_options' to base class ctor #60

merged 2 commits into from Aug 5, 2020

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Aug 5, 2020

packaging: pin 'google-cloud-core >= 1.4.0'

Closes #50.

@tseaver
feat: pass 'client_options' to base class ctor
6efd02a 
packaging: pin 'google-cloud-core >= 1.4.0'

Closes #50.
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Aug 5, 2020
chrisrossi
chrisrossi approved these changes Aug 5, 2020
View reviewed changes
Copy link
Contributor

@chrisrossi chrisrossi left a comment

Choose a reason for hiding this comment

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

I'm finding it strange that google.cloud.datastore.client.Client was already accepting a client_options argument but wasn't doing anything with it. LGTM, other than confusion about the preceding state of the code.

@tseaver tseaver added the automerge Merge the pull request once unit tests and other checks pass. label Aug 5, 2020
@tseaver
Copy link
Contributor Author

tseaver commented Aug 5, 2020

@chrisrossi If you look below the super call in the method, the constructor used / uses client_options to find an overridden api_endpoint URL.

@gcf-merge-on-green gcf-merge-on-green bot merged commit 2575697 into master Aug 5, 2020
@tseaver tseaver deleted the 50-pass-client_options-to-base-ctor branch August 5, 2020 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisrossi chrisrossi chrisrossi approved these changes

@cguardia cguardia Awaiting requested review from cguardia

@busunkim96 busunkim96 Awaiting requested review from busunkim96

Assignees
No one assigned
Labels
automerge Merge the pull request once unit tests and other checks pass. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement new client options
2 participants
@tseaver @chrisrossi