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

feat: Use float instead of int for GCE_METADATA_TIMEOUT #1481

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ludoch
Copy link

@ludoch ludoch commented Feb 20, 2024

Move the logic to read GCE_METADATA_TIMEOUT env variable so we can better control it in app_identity component.

Related internal change is 607513254.

Move the logic to read GCE_METADATA_TIMEOUT env variable so we can better control it in app_identity component.

Related internal change is 607513254.
@ludoch ludoch requested review from a team as code owners February 20, 2024 20:02
Copy link

conventional-commit-lint-gcf bot commented Feb 20, 2024

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@estrellis
Copy link

The change LGTM

@ludoch ludoch changed the title Move the logic to read GCE_METADATA_TIMEOUT. Update _metadata.py Feb 21, 2024
@clundin25 clundin25 changed the title Update _metadata.py feat: Use float instead of int for GCE_METADATA_TIMEOUT Feb 21, 2024
Comment on lines 121 to 122
except ValueError: # pragma: NO COVER
pass
Copy link
Contributor

@clundin25 clundin25 Feb 21, 2024

Choose a reason for hiding this comment

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

I would not capture a ValueError here. Additionally, the timeout variable would not be usable, so the downstream call would have problems.

Choose a reason for hiding this comment

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

FWIW the original code caught the ValueError. The value is set by the user.
I am thinking if the Exception happens timeout will retain the passed in/default value

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case, can you remove the pragma, and include test coverage for this change?

judahrand added a commit to judahrand/google-auth-library-python that referenced this pull request Mar 2, 2024
ludoch and others added 4 commits March 20, 2024 17:44
* Add a test with new _metadata.py changes

* Add tests for  metadata changes.
* chore(python): update dependencies in /.kokoro (googleapis#1499)

Source-Link: googleapis/synthtool@db94845
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:a8a80fc6456e433df53fc2a0d72ca0345db0ddefb409f1b75b118dfd1babd952

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <[email protected]>

* Add a test with new _metadata.py changes

* Add tests for  metadata changes.

* chore(python): bump idna from 3.4 to 3.7 in .kokoro (googleapis#1517)

Source-Link: googleapis/synthtool@d50980e
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:5a4c19d17e597b92d786e569be101e636c9c2817731f80a5adec56b2aa8fe070

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <[email protected]>

* Update environment_vars.py

---------

Co-authored-by: gcf-owl-bot[bot] <78513119+gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants