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: Implement asynchronous AuthorizedSession api request class #1579

Conversation

ohmayr
Copy link
Contributor

@ohmayr ohmayr commented Aug 11, 2024

This class implements the following:

  • an interface for making asynchronous requests which is consumed by the AuthorizedSession API i.e. google.auth.aio.transport.Request.
  • a concrete class which leverages aiohttp.ClientSession to make asynchronous requests i.e. google.auth.aio.transport.aiohttp.Request.

@ohmayr ohmayr changed the base branch from main to add-support-for-async-authorized-session-api August 11, 2024 20:24
Comment on lines 55 to 57
# TODO(ohmayr): Evaluate if we want auto_decompress=False.
# and if we want to update it in the passed in session.
self.session = session or aiohttp.ClientSession(auto_decompress=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just use the defaults, and if anyone needs to set a custom parameter they can pass in their own session.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me. I think we can work around by not setting auto_decompress so it should be fine or come back to it later if always needed.

What I'm unsure about is why trust_env is set here as False.

By default the value is set to True i.e. it'll automatically pull up any proxy settings in the environment unlike what we've been doing in our experimental code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to correct myself here. The default value for trust_env is False.

will punt adding this as a parameter until required. Thanks!

@ohmayr ohmayr changed the title feat: Implement Asynchronous AuthorizedSession API Request Class feat: Implement asynchronous AuthorizedSession api request class Aug 13, 2024
@ohmayr ohmayr marked this pull request as ready for review August 13, 2024 07:23
@ohmayr ohmayr requested review from a team as code owners August 13, 2024 07:23
url: str,
method: Optional[str],
body: bytes,
headers: Mapping[str, str],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@clundin25
Do we want to change the Response.headers type to a Mapping instead of a Dict to match this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If required, will be addressed in the cleanup PR towards the end.

@ohmayr ohmayr merged commit 5f46b60 into add-support-for-async-authorized-session-api Aug 13, 2024
5 of 7 checks passed
@ohmayr ohmayr deleted the implement-async-auth-session-api-request-class branch August 13, 2024 17:23
ohmayr added a commit that referenced this pull request Sep 16, 2024
* chore: initial setup for async auth sessions api (#1571)

* chore: initial setup for async auth sessions api

* fix whitespace

* add init file

* update file names to aiohttp

* update import statement

* feat: Implement asynchronous timeout context manager (#1569)

* feat: implement async timeout guard

* add docstring

* clean whitespace

* update import file name

* add missing return statement

* update test cases

* update test cases

* include underlying timeout exception in trace

* avoid the cost of actual time

* feat: Implement asynchronous `AuthorizedSession` api response class (#1575)

* feat: implement asynchronous response class for AuthorizedSessions API

* check if aiohttp is installed and avoid tests dependency

* update content to be async

* update docstring to be specific to aiohttp

* add type checking and avoid leaking underlying API responses

* add test case for iterating chunks

* add read method to response interface

* address PR comments

* fix lint issues

* feat: Implement asynchronous `AuthorizedSession` api request class (#1579)

* feat: implement request class for asynchoronous AuthorizedSession API

* add type checking and address TODOs

* remove default values from interface methods

* aiohttp reponse close method must not be awaited

* cleanup

* update Request class docstring

* feat: Implement asynchronous `AuthorizedSession` class (#1580)

* feat: Implement Asynchronous AuthorizedSession class

* add comment for implementing locks within refresh

* move timeout guard to sessions

* add unit tests and code cleanup

* implement async exponential backoff iterator

* cleanup

* add testing for http methods and cleanup

* update number of retries to 3

* refactor test cases

* fix linter and mypy issues

* fix pytest code coverage

* fix: avoid leaking api error for closed session

* add error handling for response

* cleanup default values and add test coverage

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* cleanup: minor code cleanup (#1589)

* chore: Add aiohttp requirements test constraint. (#1566)

See #1565
for more information.

* chore(main): release 2.33.0 (#1560)

* chore(main): release 2.33.0

* fix: retry token request on retryable status code (#1563)

* fix: retry token request on retryable status code

* feat(auth): Update get_client_ssl_credentials to support X.509 workload certs (#1558)

* feat(auth): Update get_client_ssl_credentials to support X.509 workload certs

* feat(auth): Update has_default_client_cert_source

* feat(auth): Fix formatting

* feat(auth): Fix test__mtls_helper.py

* feat(auth): Fix function name in tests

* chore: Refresh system test creds.

* feat(auth): Fix style

* feat(auth): Fix casing

* feat(auth): Fix linter issue

* feat(auth): Fix coverage issue

---------

Co-authored-by: Carl Lundin <[email protected]>
Co-authored-by: Carl Lundin <[email protected]>

* chore: Update ECP deps. (#1583)

* chore(main): release 2.34.0 (#1574)

* cleanup: minor code cleanup

* fix lint issues

---------

Co-authored-by: Carl Lundin <[email protected]>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Andy Zhao <[email protected]>
Co-authored-by: Carl Lundin <[email protected]>

* update secrets from forked repo

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: arithmetic1728 <[email protected]>
Co-authored-by: Carl Lundin <[email protected]>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Andy Zhao <[email protected]>
Co-authored-by: Carl Lundin <[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.

2 participants