FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Adding optional switch to capture project ID in from_service_account_json(). by dhermes · Pull Request #3436 · googleapis/google-cloud-python · GitHub

Adding optional switch to capture project ID in from_service_account_json(). - #3436

Merged
dhermes merged 1 commit into
googleapis:masterfrom
dhermes:fix-1883
Jun 6, 2017
Merged

Adding optional switch to capture project ID in from_service_account_json().#3436
dhermes merged 1 commit into
googleapis:masterfrom
dhermes:fix-1883

Conversation

dhermes commented May 17, 2017

Copy link
Copy Markdown
Contributor

Fixes #1883.

@lukesneeringer This is a long-standing bug but should really make it into a google-cloud-core release. I know you hate releasing core.

googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 17, 2017

theacodes commented May 17, 2017
edited by dhermes
Loading

Copy link
Copy Markdown
Contributor

It would be great to solve this by just consolidating ClientWithProject and ClientWithCredentials Client

dhermes commented May 17, 2017

Copy link
Copy Markdown
Contributor Author

@jonparrott See the table I made about usage. The only issue with that is some APIs don't use a project (and we don't want to fail their constructors over a missing project).

theacodes commented May 17, 2017 via email
edited by dhermes
Loading

Copy link
Copy Markdown
Contributor

dhermes commented May 17, 2017

Copy link
Copy Markdown
Contributor Author

@jonparrott Mostly it doesn't help because it isn't true?

Or do you mean something else?

theacodes commented May 17, 2017 via email
edited by dhermes
Loading

Copy link
Copy Markdown
Contributor

dhermes commented May 17, 2017

Copy link
Copy Markdown
Contributor Author

But there is no google.auth.default_project?

theacodes commented May 17, 2017 via email
edited by dhermes
Loading

Copy link
Copy Markdown
Contributor

dhermes commented May 17, 2017

Copy link
Copy Markdown
Contributor Author

Makes sense.

@lukesneeringer WDYT of us just using project on every Client instance? (This would just bring Client and ClientWithProject together, ClientWithProject is already a tiny impl.)

tseaver commented May 18, 2017
edited
Loading

Copy link
Copy Markdown
Contributor

FTR, ClientWithProject is skipped by the following:

$ for f in $(ls */google/cloud/*/client.py); do
       if ! grep -q ClientWithProject $f; then
           echo $f
       fi
  done
bigtable/google/cloud/bigtable/client.py
language/google/cloud/language/client.py
resource_manager/google/cloud/resource_manager/client.py
spanner/google/cloud/spanner/client.py
speech/google/cloud/speech/client.py
translate/google/cloud/translate/client.py

Of those, the following have _ClientProjectMixin included:

$ for f in $(ls */google/cloud/*/client.py); do
       if ! grep -q ClientWithProject $f; then
           echo $f
       fi
  done | xargs grep -l _ClientProjectMixin
bigtable/google/cloud/bigtable/client.py
spanner/google/cloud/spanner/client.py

dhermes commented May 19, 2017

Copy link
Copy Markdown
Contributor Author

@tseaver This is (intended to be) captured in my table

# Check that mocks were called as expected.
file_open.assert_called_once_with(
mock.sentinel.filename, 'r', encoding='utf-8')
constructor.assert_called_once_with(info)

This comment was marked as spam.

This comment was marked as spam.

dhermes merged commit a55010f into googleapis:master Jun 6, 2017
dhermes deleted the fix-1883 branch June 6, 2017 17:12

dhermes commented Jun 6, 2017

Copy link
Copy Markdown
Contributor Author

@tseaver Sorry I missed your LGTM 18 days ago.

It's good I got this in, in case we have a google-cloud-core release for your gRPC remap PR, this can go in too.

landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 21, 2017
landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017
landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017
parthea pushed a commit that referenced this pull request Oct 21, 2023
…rm/python-docs-samples#3436)

* [container_registry] fix: fix broken test

fixes #3435

* Use Pub/Sub message receiver that can notify main thread
  when it has received expected number of messages.
* Only test one single occurence.
* Use uuid4 wherever makes sense.
* test if Pub/Sub client receives at least one message
parthea pushed a commit that referenced this pull request Nov 22, 2025
parthea pushed a commit that referenced this pull request Nov 24, 2025
parthea pushed a commit that referenced this pull request Mar 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: core cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL