feat: add _async_resumable_media experimental support #179

crwilcox · 2020-09-29T19:54:40Z

This is a merge of multiple previously reviewed PRs
#178
#176
#153

It says 9k lines, keep in mind, technically this is all reviewed code. I think areas to focus on are changes to existing public interfaces, as the async code should all be boxed in an internal area. This is setup.py and noxfile.py

Sets up the directory structure by copying over files needed for async. This will make reviewing async features easier.

* fix(docs-presubmit): changes needed for docs-presubmit build * docs: update docs build (via synth) By manually running synthtool * add docfx nox session

* refactor: use named consts for session Python versions Closes googleapis#150.

Co-authored-by: Tres Seaver <tseaver@palladion.com>

Prepare for upcoming version, which will namespace the module to avoid conflict with the ICRAR version. See: googleapis/python-crc32c#29

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Tres Seaver <tseaver@palladion.com>

Co-authored-by: Anirudh Baddepuddi <anibadde@google.com>

…auth.default_async, aiohttp_requests (googleapis#176)

…apis#178)

…-python into async

google-cla · 2020-09-29T19:54:55Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2020-09-29T19:58:29Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2020-09-29T19:59:34Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2020-09-29T20:04:25Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

…in branch

google-cla · 2020-09-29T20:07:15Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2020-09-29T20:22:17Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2020-09-29T20:35:32Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

crwilcox · 2020-09-30T20:33:47Z

google/_async_resumable_media/requests/_request_helpers.py

+from google._async_resumable_media import _helpers
+from google.resumable_media import common
+
+import google.auth.transport._aiohttp_requests as aiohttp_requests


This, which is used to allow for a combined response, has a hard dependency on google-auth. We don't currently have one for the package.

Should google-auth be re-added as an explicit dependency?

I think it is safe to do so. It is unlikely this library is used without google-auth or something else that requires it (api-core for instance).

@frankyn @tswast any opinions?

#180 I made thinking we might need this.

I sort of think refactoring to avoid the dependency may be futile since folks will have the dependency anyway?

@crwilcox I agree - if most folks are going to have to install google-auth I'd prefer to have it in the requirements.

crwilcox · 2020-09-30T20:34:47Z

google/_async_resumable_media/requests/_request_helpers.py

+from google.resumable_media import common
+
+import google.auth.transport._aiohttp_requests as aiohttp_requests
+import aiohttp


requests is an optional extra, but due to the use of an aiohttp specific timeout, we have to make an explicit dependency. This could be refactored to be an extra and fail with a good message if not installed.

Given the conversation on googleapis/google-auth-library-python#618 I think we should keep aiohttp an extra for the immediate term, since we aren't exposing users to it.

busunkim96 · 2020-09-30T20:54:41Z

noxfile.py looks good to me, just curious about the dependencies needed in setup.py.

google-cla · 2020-10-05T15:53:52Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

andrewsg

Due to the length of the PR and the various thorough previous incremental reviews I had to skim this one. Thanks for all your hard work on this.

tests_async/system/credentials.json.enc

tritone and others added 10 commits August 3, 2020 11:26

feat: initial async commit

4b0f97e

Sets up the directory structure by copying over files needed for async. This will make reviewing async features easier.

docs: update docs build (via synth) (googleapis#155)

8aede77

* fix(docs-presubmit): changes needed for docs-presubmit build * docs: update docs build (via synth) By manually running synthtool * add docfx nox session

tests: harden systest teardowns vs transient GCS errors (googleapis#158)

094938b

* refactor: use named consts for session Python versions Closes googleapis#150.

chore(deps): update ubuntu docker tag to v20.10 (googleapis#156)

b48a834

Co-authored-by: Tres Seaver <tseaver@palladion.com>

chore(deps): pin 'google-crc32c < 0.2dev' (googleapis#160)

8878754

Prepare for upcoming version, which will namespace the module to avoid conflict with the ICRAR version. See: googleapis/python-crc32c#29

chore: release 0.7.1 (googleapis#157)

35881fa

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Tres Seaver <tseaver@palladion.com>

feat: async changes to resumable upload/download (googleapis#153)

190a62c

Co-authored-by: Anirudh Baddepuddi <anibadde@google.com>

chore(deps): use google-auth >= 1.22.0, 2.0dev which includes google.…

3f9044d

…auth.default_async, aiohttp_requests (googleapis#176)

fix: move async_resumable_media to _async_resumable_media (google…

e5fd1cc

…apis#178)

Merge branch 'master' of github.com:googleapis/google-resumable-media…

80a1bf4

…-python into async

crwilcox requested a review from a team as a code owner September 29, 2020 19:54

crwilcox added the cla: yes This human has signed the Contributor License Agreement. label Sep 29, 2020

test: missed merge, noxfile

e01cfbf

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

test: missed merge, noxfile

6894f71

crwilcox added the cla: yes This human has signed the Contributor License Agreement. label Sep 29, 2020

google-cla bot removed the cla: no This human has *not* signed the Contributor License Agreement. label Sep 29, 2020

chore(deps): match async branch dependency on google-auth to match ma…

13ede91

…in branch

crwilcox force-pushed the merge-async branch from ccf46a9 to 13ede91 Compare September 29, 2020 20:07

google-cla bot removed the cla: yes This human has signed the Contributor License Agreement. label Sep 29, 2020

crwilcox 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 Sep 29, 2020

test: erroneous comma in const

56ac9dc

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

crwilcox 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 Sep 29, 2020

test: remove duplicate constants from merge

e37ab74

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

crwilcox 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 Sep 29, 2020

crwilcox requested review from andrewsg, busunkim96 and tswast September 30, 2020 20:12

crwilcox commented Sep 30, 2020

View reviewed changes

fix: move aiohttp to extra as surface is still internal

d3a3665

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

andrewsg approved these changes Oct 5, 2020

View reviewed changes

tests_async/system/credentials.json.enc Show resolved Hide resolved

crwilcox self-assigned this Oct 5, 2020

crwilcox added cla: yes This human has signed the Contributor License Agreement. kokoro:run Add this label to force Kokoro to re-run the tests. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Oct 5, 2020

crwilcox mentioned this pull request Oct 5, 2020

Reduce duplication for tests suite files #181

Open

crwilcox merged commit 03c11ba into googleapis:master Oct 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add _async_resumable_media experimental support #179

feat: add _async_resumable_media experimental support #179

crwilcox commented Sep 29, 2020 •

edited

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

crwilcox Sep 30, 2020

busunkim96 Sep 30, 2020

crwilcox Sep 30, 2020

crwilcox Sep 30, 2020

busunkim96 Sep 30, 2020

crwilcox Sep 30, 2020 •

edited

crwilcox Oct 5, 2020

andrewsg Oct 5, 2020

busunkim96 commented Sep 30, 2020

google-cla bot commented Oct 5, 2020

andrewsg left a comment

feat: add _async_resumable_media experimental support #179

feat: add _async_resumable_media experimental support #179

Conversation

crwilcox commented Sep 29, 2020 • edited

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

google-cla bot commented Sep 29, 2020

crwilcox Sep 30, 2020

Choose a reason for hiding this comment

busunkim96 Sep 30, 2020

Choose a reason for hiding this comment

crwilcox Sep 30, 2020

Choose a reason for hiding this comment

crwilcox Sep 30, 2020

Choose a reason for hiding this comment

busunkim96 Sep 30, 2020

Choose a reason for hiding this comment

crwilcox Sep 30, 2020 • edited

Choose a reason for hiding this comment

crwilcox Oct 5, 2020

Choose a reason for hiding this comment

andrewsg Oct 5, 2020

Choose a reason for hiding this comment

busunkim96 commented Sep 30, 2020

google-cla bot commented Oct 5, 2020

andrewsg left a comment

Choose a reason for hiding this comment

crwilcox commented Sep 29, 2020 •

edited

crwilcox Sep 30, 2020 •

edited