feat: added system tests for the asyncIO auth changes and async id_token credentials #574

anibadde · 2020-07-28T02:40:58Z

PR # 3 for AsyncIO Auth Class changes

Added in system tests paralleling existing regular system tests for the async implementation of the auth library and credentials
Fixed Timeout Context Manager bug that enables connection to asynchronous resumable media implementation.
Added in async id token credentials logic along with defaul, service account systems.
Required for credential depending on existence/validity of service account file

google-cla · 2020-07-28T02:41:05Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

anibadde · 2020-07-28T15:10:34Z

@googlebot I signed it!

anibadde · 2020-07-28T15:10:52Z

@googlebot I signed it!

anibadde · 2020-07-28T17:37:41Z

@googlebot I signed it!

…ext manager

…n class

…p_requests

…onship

tritone

Good progress! I think there's definitely some de-duplication that can happen in the nox and pytest configurations; @crwilcox might have more ideas. I'm also curious to hear other's thoughts on whether more system tests should be added.

tritone · 2020-07-31T18:12:29Z

google/oauth2/id_token_async.py

+
+    session = requests.session()
+    cached_session = cachecontrol.CacheControl(session)
+    request = google.auth.transport.requests.Request(session=cached_session)


Would this be the same for async?

I made this specific request call sync as an async version of this would result in a substantial re-working of the compute_engine credentials classes (as the current design is not compatible with asynchronous http requests). My reasoning for this was that this authentication trace isn't used in storage, but we could open up an issue for it as a future modification to make for the auth library.

Please open a tracking bug

Ok will do.

tritone · 2020-07-31T18:13:21Z

google/oauth2/id_token_async.py

+            "Could not fetch certificates at {}".format(certs_url)
+        )
+
+    data = await response.data.read()


Is this a network op?

Yes, response.data returns a stream coroutine, and therefore needs to be awaited in order to read the content of the response.

google/oauth2/id_token_async.py

tritone · 2020-07-31T18:22:49Z

system_tests_async/conftest.py

+
+
+HERE = os.path.dirname(__file__)
+DATA_DIR = os.path.join(HERE, "data")


So it seems like this will require the data to be re-copied into system_tests_async/data as well? Could we avoid that by using the same data dir for both sets of system tests?

I thought about this, since the data files aren't in the repository and are there for the user to run locally, I thought that it may make sense to leave this logic as is (without a dependency on the sync system tests) in case someone wanted to import the data files just for the async use case, similar to what I did when I was working on this and running tests locally. I think that regardless of whether we change the directory name, we would have to follow the steps in https://github.com/googleapis/google-auth-library-python/blob/master/CONTRIBUTING.rst to be able to run these tests.

I'm open to changing the directory if that would be better, but I thought it would be good to avoid the dependency in this case?

tritone · 2020-07-31T18:23:44Z

system_tests_async/conftest.py

+
+
+@pytest.fixture
+def service_account_file():


If these are copied from system_tests/conftest.py, could they be moved to the root conftest.py to de-duplicate?

Same inter-dependency reasoning as above, open to changing if we are okay with having the dependency.

system_tests_async/noxfile.py

system_tests_async/test_default.py

system_tests_async/test_service_account.py

tests_async/oauth2/test_id_token.py

tritone · 2020-07-31T18:34:25Z

system_tests_async/test_service_account.py

+            "https://www.googleapis.com/auth/userinfo.email",
+            "https://www.googleapis.com/auth/userinfo.profile",
+        ]
+    )


So I see 2 system tests in this file, and 1 for id token. Does the sync version have additional system tests that you did not copy? If so, why?

The sync version also has 1 system test for id_token and 2 for service accounts, so a parallel code trace (same corresponding tests) are used here as in the sync system tests.

google/oauth2/id_token_async.py

system_tests_async/noxfile.py

anibadde added 3 commits July 27, 2020 20:53

feat: asyncio http request logic and asynchronous credentials log c

ef6d872

feat: all asynchronous credentials types implemented and with tests

0a63b3c

included system tests for the asynchronous auth library

1fb2bce

google-cla bot added the cla: no This human has *not* signed the Contributor License Agreement. label Jul 28, 2020

tritone requested review from crwilcox, busunkim96, andrewsg and frankyn and removed request for crwilcox July 28, 2020 15:07

google-cla bot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Jul 28, 2020

frankyn changed the base branch from master to async July 28, 2020 16:52

anibadde added 13 commits July 28, 2020 12:39

feat: added the private scope for Response class

06606ff

feat: added docstring for Auth Session request method

cd57fad

fix: Changed initialization of client session to within an async cont…

a38e333

…ext manager

changed aiohttp_requests abbreviation for the async authorized sessio…

57d6d10

…n class

Merge branch 'auth1' into auth2

1db84f2

fix: changed abbrevation of the aiohttp_requests file

cc14082

Merge branch 'auth2' into auth3

80d005f

fix: comments on PR regarding shared data between requests and aiohtt…

aa6ece2

…p_requests

fix: fixed noxfile test dependency sharing

d88839b

fix: Made fixes based on comments on first PR

dfc6251

fix: fixed the noxfile dependencies between sync and async unit tests

c62dd1a

fix: cover async dependency

7080d14

Merge branch 'auth1' into auth2

8122dbb

anibadde added 10 commits July 28, 2020 18:13

Merge branch 'async' into auth2

77ca4c7

fix: merge conflict issue with credentials

b4c306f

fix: merge conflict googleapis#2

9ddb911

fix: changed duplicated constants for sync-->async inheritance relati…

8f2d0ef

…onship

fix: async docstring

9ec8277

refactoring

8f254de

fix: refactoring

aa04dc9

fix: first round of comments, refactoring and test duplication changes

0c4c3b6

fix: removed duplication in _default_async

92175f2

Merge branch 'auth2' into auth3

2b29a54

tseaver mentioned this pull request Jul 30, 2020

feat: asyncio support and aiohttp transport #465

Closed

anibadde added 2 commits July 30, 2020 17:52

compute engine and metadata changes

337c772

fix: removed oauth2 client

77d7b6e

busunkim96 closed this Jul 31, 2020

added further system tests and refactored

fd30685

busunkim96 reopened this Jul 31, 2020

anibadde added 2 commits July 31, 2020 12:46

Merge branch 'auth2' into auth3

0ba2dda

Merge branch 'async' into auth3

50f7fb7

anibadde changed the title ~~feat: added system tests for the asyncIO auth changes~~ feat: added system tests for the asyncIO auth changes and async id_token credentials Jul 31, 2020

tritone reviewed Jul 31, 2020

View reviewed changes

anibadde added 2 commits July 31, 2020 13:51

modified aiohttp request docstring

1d7c8dc

refactoring and fixing comments

f7b49ed

crwilcox suggested changes Jul 31, 2020

View reviewed changes

google/oauth2/id_token_async.py Show resolved Hide resolved

google/oauth2/id_token_async.py Outdated Show resolved Hide resolved

system_tests_async/noxfile.py Outdated Show resolved Hide resolved

anibadde added 4 commits July 31, 2020 18:00

refactored system tests and re-wrote nox file

dd92972

metadata typo

c8a61c0

fix: nox file tests added

53567f2

fix: directory path in app_engine

1ed6e11

crwilcox approved these changes Jul 31, 2020

View reviewed changes

crwilcox merged commit 0576ed3 into googleapis:async Jul 31, 2020

crwilcox mentioned this pull request Sep 25, 2020

feat: add asyncio based auth flow #612

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: added system tests for the asyncIO auth changes and async id_token credentials #574

feat: added system tests for the asyncIO auth changes and async id_token credentials #574

anibadde commented Jul 28, 2020 •

edited

Loading

google-cla bot commented Jul 28, 2020

anibadde commented Jul 28, 2020

anibadde commented Jul 28, 2020

anibadde commented Jul 28, 2020

tritone left a comment

tritone Jul 31, 2020

anibadde Jul 31, 2020

crwilcox Jul 31, 2020

anibadde Jul 31, 2020

tritone Jul 31, 2020

anibadde Jul 31, 2020

tritone Jul 31, 2020

anibadde Jul 31, 2020

tritone Jul 31, 2020

anibadde Jul 31, 2020

tritone Jul 31, 2020

anibadde Jul 31, 2020



		HERE = os.path.dirname(__file__)
		DATA_DIR = os.path.join(HERE, "data")



		@pytest.fixture
		def service_account_file():

feat: added system tests for the asyncIO auth changes and async id_token credentials #574

feat: added system tests for the asyncIO auth changes and async id_token credentials #574

Conversation

anibadde commented Jul 28, 2020 • edited Loading

google-cla bot commented Jul 28, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

anibadde commented Jul 28, 2020

anibadde commented Jul 28, 2020

anibadde commented Jul 28, 2020

tritone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anibadde commented Jul 28, 2020 •

edited

Loading