Adding scope to credentials in Connection constructors.#840
Adding scope to credentials in Connection constructors.#840dhermes merged 1 commit intogoogleapis:masterfrom
Conversation
|
I might be misremembering, but it feels like we've been around a complete circle here. |
|
What's the circle? I don't recall the |
|
6ebc65e moved the scope handling from |
|
The idea behind this commit is not just churn / moving the code. We want people to be able to get credentials from elsewhere and pass them in to a connection without worrying about adding the scopes. If we never expect people to construct a This was all brought about by @jgeewax pointing out some alternate auth / credentials flows. |
|
@tseaver Can we keep moving on this? |
gcloud/connection.py
Outdated
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
If I recall correctly, the difference this makes becomes apparent when you look at the case where you want to use some credentials from a specific file to talk to a service. Old way: from gcloud.credentials import get_for_service_account_json
from gcloud import pubsub
credentials = get_for_service_account_json('/path/to/key.json')
credentials = credentials.create_scoped(pubsub.SCOPE) # <-- The thing I don't want to have to type...
connection = pubsub.Connection(credentials=credentials)
pubsub.set_default_connection(connection)
pubsub.Topic('topic_name').create()(Also note that in New way from gcloud.credentials import get_for_service_account_json
from gcloud import pubsub
credentials = get_for_service_account_json('/path/to/key.json')
connection = pubsub.Connection(credentials=credentials) # Scoping happens here, where we know we need it...
pubsub.set_default_connection(connection)
pubsub.Topic('topic_name').create()Is there a reason why you'd want to keep the "scope credentials" action separate from the "create connection" action? I can't think of a reason but maybe you guys know more about this... If there isn't then I fully support this change -- seems much more convenient to do this when creating the connection (and expecting the connection to know which scopes are necessary on the credentials you supplied). |
|
I also fully support this change FWIW |
|
@tseaver PTAL |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
0cb6fc9 to
7cfc884
Compare
|
Just rebased on top of @tseaver Can we resolve the discussion and move forward here? |
|
@tseaver Can we wrap this up? |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
LGTM, module my comments this morning on the |
This also obsoletes get_scoped_connection() so it is removed.
7cfc884 to
b44b25b
Compare
Adding scope to credentials in Connection constructors.
Source-Link: googleapis/synthtool@5f2a608 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:8555f0e37e6261408f792bfd6635102d2da5ad73f8f09bcb24f25e6afb5fac97 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* chore: create flakybot.yaml to change default issue priority * add googlel copyright license --------- Co-authored-by: cindy-peng <cindypeng@google.com>
Source-Link: googleapis/synthtool@fac8444 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:5ea6d0ab82c956b50962f91d94e206d3921537ae5fe1549ec5326381d8905cfa Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Source-Link: googleapis/synthtool@fac8444 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:5ea6d0ab82c956b50962f91d94e206d3921537ae5fe1549ec5326381d8905cfa Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* chore: release 2.0.0 * Update CHANGELOG.md * chore: set version number to 2.0.0 Follow up to #829 Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
* test: refactor `list_rows` tests and add test for scalars * WIP: INTERVAL support * feat: add support for INTERVAL data type to `list_rows` * fix relativedelta construction for non-microseconds * WIP: support INTERVAL query params * remove dead code * INTERVAL not supported in query parameters * revert query parameter changes * add validation error for interval * add unit tests for extreme intervals * add dateutil to intersphinx * use dictionary for intersphinx * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * add test case for trailing . * explicit none * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * truncate nanoseconds * use \d group for digits * use \d for consistency Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Peter Lamut <plamut@users.noreply.github.com>
Source-Link: googleapis/synthtool@d52e638 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:4f9b3b106ad0beafc2c8a415e3f62c1a0cc23cabea115dbe841b848f581cfe99 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
This also obsoletes get_scoped_connection() so it is removed.
FYI @jgeewax thanks for nudging in this direction. It moves the scope adding behavior into the
Connectionconstructors.