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

Include the GCP scope when creating a scoped credential token. #23410

Closed
wants to merge 1 commit into from

Conversation

tvalentyn
Copy link
Contributor

@tvalentyn tvalentyn commented Sep 28, 2022

Include the GCP scope when creating a scoped credential token.
@tvalentyn tvalentyn closed this Sep 28, 2022
@tvalentyn tvalentyn reopened this Sep 28, 2022
@github-actions
Copy link
Contributor

The Workflow run is cancelling this PR. It is an earlier duplicate of 2083803 run.

@tvalentyn
Copy link
Contributor Author

R: @chamikaramj

@tvalentyn tvalentyn changed the title Update auth.py Include the GCP scope when creating a scoped credential token. Sep 28, 2022
@github-actions
Copy link
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

@tvalentyn
Copy link
Contributor Author

Run Python 3.8 postcommit

@tvalentyn
Copy link
Contributor Author

R: @ricardograca-scratch

@chamikaramj
Copy link
Contributor

LGTM. Thanks.

@chamikaramj
Copy link
Contributor

(also mention in the PR description that this scope is already included for Java SDK).

@codecov
Copy link

codecov bot commented Sep 28, 2022

Codecov Report

Merging #23410 (9a36f53) into master (e53466f) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #23410      +/-   ##
==========================================
- Coverage   73.41%   73.40%   -0.01%     
==========================================
  Files         718      718              
  Lines       95652    95652              
==========================================
- Hits        70222    70216       -6     
- Misses      24119    24125       +6     
  Partials     1311     1311              
Flag Coverage Δ
python 83.16% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sdks/python/apache_beam/internal/gcp/auth.py 78.66% <ø> (+5.33%) ⬆️
...ks/python/apache_beam/runners/worker/data_plane.py 87.57% <0.00%> (-1.70%) ⬇️
sdks/python/apache_beam/internal/metrics/metric.py 93.00% <0.00%> (-1.00%) ⬇️
...hon/apache_beam/runners/direct/test_stream_impl.py 93.28% <0.00%> (-0.75%) ⬇️
...eam/runners/portability/fn_api_runner/execution.py 92.44% <0.00%> (-0.65%) ⬇️
...ks/python/apache_beam/runners/worker/sdk_worker.py 88.94% <0.00%> (-0.16%) ⬇️
...hon/apache_beam/runners/worker/bundle_processor.py 93.67% <0.00%> (+0.12%) ⬆️
...eam/runners/interactive/interactive_environment.py 92.02% <0.00%> (+0.30%) ⬆️
sdks/python/apache_beam/utils/interactive_utils.py 97.56% <0.00%> (+2.43%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -45,6 +45,7 @@
_LOGGER = logging.getLogger(__name__)

CLIENT_SCOPES = [
'https://www.googleapis.com/auth/cloud-platform',
'https://www.googleapis.com/auth/bigquery',
'https://www.googleapis.com/auth/cloud-platform',
Copy link

@ricardograca-scratch ricardograca-scratch Sep 29, 2022

Choose a reason for hiding this comment

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

Looks like the scope is already there, so this PR isn't actually doing anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, you are right. This would not have any affect.

Copy link

@ricardograca-scratch ricardograca-scratch left a comment

Choose a reason for hiding this comment

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

This PR is adding a line that was already there.

@tvalentyn
Copy link
Contributor Author

oops, my bad.

@tvalentyn tvalentyn closed this Sep 29, 2022
@tvalentyn tvalentyn deleted the tvalentyn-patch-1 branch May 16, 2023 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants