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

Use client_error instead of a hard-coded exception in TCPConnector #5502

Closed
wants to merge 1 commit into from

Conversation

illia-v
Copy link
Contributor

@illia-v illia-v commented Feb 26, 2021

What do these changes do?

Use client_error that is an existing method argument instead of a hard-coded exception.

Are there changes in behavior for the user?

I doubt.
Although, _create_proxy_connection passes a different client_error and it won't be handled correctly in case of a problem. So the changes may prevent a crash.
Note, I didn't face the problem myself, I found the issue when I was working on #4451.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • Add a new news fragment into the CHANGES folder

`client_error` is an existing method argument. And it seems that it is supposed to be used in the two places instead of the hard-coded exception.
@illia-v illia-v requested a review from asvetlov as a code owner February 26, 2021 21:23
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Feb 26, 2021
@codecov
Copy link

codecov bot commented Feb 26, 2021

Codecov Report

Merging #5502 (e8323ac) into master (8c25b44) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5502   +/-   ##
=======================================
  Coverage   97.17%   97.17%           
=======================================
  Files          41       41           
  Lines        8860     8860           
  Branches     1424     1424           
=======================================
  Hits         8610     8610           
  Misses        133      133           
  Partials      117      117           
Flag Coverage Δ
unit 97.06% <100.00%> (ø)

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

Impacted Files Coverage Δ
aiohttp/connector.py 96.51% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c25b44...8305f8f. Read the comment docs.

Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Please add regression tests for this change.

@@ -985,7 +985,7 @@ def drop_exception(fut: "asyncio.Future[List[Dict[str, Any]]]") -> None:
except OSError as exc:
# in case of proxy it is not ClientProxyConnectionError
Copy link
Member

Choose a reason for hiding this comment

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

this comment probably needs to be updated

@illia-v illia-v closed this Mar 9, 2021
github-actions bot pushed a commit that referenced this pull request Jan 27, 2025
Bumps [pip](https://github.com/pypa/pip) from 24.3.1 to 25.0.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/pypa/pip/blob/main/NEWS.rst">pip's
changelog</a>.</em></p>
<blockquote>
<h1>25.0 (2025-01-26)</h1>
<h2>Deprecations and Removals</h2>
<ul>
<li>Deprecate the <code>no-python-version-warning</code> flag as it has
long done nothing
since Python 2 support was removed in pip 21.0.
(<code>[#13154](pypa/pip#13154)
&lt;https://github.com/pypa/pip/issues/13154&gt;</code>_)</li>
</ul>
<h2>Features</h2>
<ul>
<li>Prefer to display :pep:<code>639</code>
<code>License-Expression</code> in <code>pip show</code> if metadata
version is at least 2.4.
(<code>[#13112](pypa/pip#13112)
&lt;https://github.com/pypa/pip/issues/13112&gt;</code>_)</li>
<li>Support :pep:<code>639</code> <code>License-Expression</code> and
<code>License-File</code> metadata fields in JSON
output. <code>pip inspect</code> and <code>pip install --report</code>
now emit
<code>license_expression</code> and <code>license_file</code> fields in
the <code>metadata</code> object,
if the corresponding fields are present in the installed
<code>METADATA</code> file.
(<code>[#13134](pypa/pip#13134)
&lt;https://github.com/pypa/pip/issues/13134&gt;</code>_)</li>
<li>Files in the network cache will inherit the read/write permissions
of pip's cache
directory (in addition to the current user retaining read/write access).
This
enables a single cache to be shared among multiple users.
(<code>[#11012](pypa/pip#11012)
&lt;https://github.com/pypa/pip/issues/11012&gt;</code>_)</li>
<li>Return the size, along with the number, of files cleared on
<code>pip cache purge</code> and <code>pip cache remove</code>
(<code>[#12176](pypa/pip#12176)
&lt;https://github.com/pypa/pip/issues/12176&gt;</code>_)</li>
<li>Cache <code>python-requires</code> checks while filtering potential
installation candidates.
(<code>[#13128](pypa/pip#13128)
&lt;https://github.com/pypa/pip/issues/13128&gt;</code>_)</li>
<li>Optimize package collection by avoiding unnecessary URL parsing and
other processing.
(<code>[#13132](pypa/pip#13132)
&lt;https://github.com/pypa/pip/issues/13132&gt;</code>_)</li>
</ul>
<h2>Bug Fixes</h2>
<ul>
<li>Reorder the encoding detection when decoding a requirements file,
relying on
UTF-8 over the locale encoding by default, matching the documented
behaviour.
(<code>[#12771](pypa/pip#12771)
&lt;https://github.com/pypa/pip/issues/12771&gt;</code>_)</li>
<li>The pip version self check is disabled on
<code>EXTERNALLY-MANAGED</code> environments.
(<code>[#11820](pypa/pip#11820)
&lt;https://github.com/pypa/pip/issues/11820&gt;</code>_)</li>
<li>Fix a security bug allowing a specially crafted wheel to execute
code during
installation. (<code>[#13079](pypa/pip#13079)
&lt;https://github.com/pypa/pip/issues/13079&gt;</code>_)</li>
<li>The inclusion of <code>packaging</code> 24.2 changes how pre-release
specifiers with <code>&lt;</code> and <code>&gt;</code>
behave. Including a pre-release version with these specifiers now
implies
accepting pre-releases (e.g., <code>&lt;2.0dev</code> can include
<code>1.0rc1</code>). To avoid
implying pre-releases, avoid specifying them (e.g., use
<code>&lt;2.0</code>).
The exception is <code>!=</code>, which never implies pre-releases.
(<code>[#13163](pypa/pip#13163)
&lt;https://github.com/pypa/pip/issues/13163&gt;</code>_)</li>
<li>The <code>--cert</code> and <code>--client-cert</code> command-line
options are now respected while
installing build dependencies. Consequently, the private
<code>_PIP_STANDALONE_CERT</code>
environment variable is no longer used.
(<code>[#5502](pypa/pip#5502)
&lt;https://github.com/pypa/pip/issues/5502&gt;</code>_)</li>
<li>The <code>--proxy</code> command-line option is now respected while
installing build dependencies.
(<code>[#6018](pypa/pip#6018)
&lt;https://github.com/pypa/pip/issues/6018&gt;</code>_)</li>
</ul>
<h2>Vendored Libraries</h2>
<ul>
<li>Upgrade CacheControl to 0.14.1</li>
<li>Upgrade idna to 3.10</li>
<li>Upgrade msgpack to 1.1.0</li>
<li>Upgrade packaging to 24.2</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/pypa/pip/commit/f47b5874299848c688336ae7c8d69534013fe2c6"><code>f47b587</code></a>
Bump for release</li>
<li><a
href="https://github.com/pypa/pip/commit/74a7f3335338712af44be95241daf62e756f27ec"><code>74a7f33</code></a>
Update AUTHORS.txt</li>
<li><a
href="https://github.com/pypa/pip/commit/a008888a5b123e8d5e4667bdd21e4b42f3fc034c"><code>a008888</code></a>
Merge pull request <a
href="https://redirect.github.com/pypa/pip/issues/13171">#13171</a> from
pypa/dependabot/github_actions/github-actions-...</li>
<li><a
href="https://github.com/pypa/pip/commit/d265fb7427c3ba4dbd10e4874a0bebea2e59350e"><code>d265fb7</code></a>
Merge pull request <a
href="https://redirect.github.com/pypa/pip/issues/13174">#13174</a> from
ichard26/changelog</li>
<li><a
href="https://github.com/pypa/pip/commit/d35384ef91cb372a5223a01f980e5deb84c8fde5"><code>d35384e</code></a>
Copyedit news entries before 25.0</li>
<li><a
href="https://github.com/pypa/pip/commit/adc4f9951b51b6a06e405b8960dd0c5f030f0fb5"><code>adc4f99</code></a>
Reorder requirements file decoding (<a
href="https://redirect.github.com/pypa/pip/issues/12795">#12795</a>)</li>
<li><a
href="https://github.com/pypa/pip/commit/40c42149a51a63e8416c047d5ddc0da1694387ea"><code>40c4214</code></a>
Bump pypa/gh-action-pypi-publish in the github-actions group</li>
<li><a
href="https://github.com/pypa/pip/commit/6b0fb904803fbb3ce7da63966b2759407b6cd9dc"><code>6b0fb90</code></a>
Merge pull request <a
href="https://redirect.github.com/pypa/pip/issues/13048">#13048</a> from
sbidoul/trusted-publisher-sbi</li>
<li><a
href="https://github.com/pypa/pip/commit/c7fb1e13ec79b1b48481ac245144c2b368e64f7d"><code>c7fb1e1</code></a>
Merge pull request <a
href="https://redirect.github.com/pypa/pip/issues/13145">#13145</a> from
befeleme/pip-show-pep639</li>
<li><a
href="https://github.com/pypa/pip/commit/41c807c5938d269703c6ff2644fb3b7dc88eda4e"><code>41c807c</code></a>
Show License-Expression if present in package metadata</li>
<li>Additional commits viewable in <a
href="https://github.com/pypa/pip/compare/24.3.1...25.0">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pip&package-manager=pip&previous-version=24.3.1&new-version=25.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
github-actions bot pushed a commit that referenced this pull request Jan 28, 2025
Bumps [pip](https://github.com/pypa/pip) from 24.3.1 to 25.0.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/pypa/pip/blob/main/NEWS.rst">pip's
changelog</a>.</em></p>
<blockquote>
<h1>25.0 (2025-01-26)</h1>
<h2>Deprecations and Removals</h2>
<ul>
<li>Deprecate the <code>no-python-version-warning</code> flag as it has
long done nothing
since Python 2 support was removed in pip 21.0.
(<code>[#13154](pypa/pip#13154)
&lt;https://github.com/pypa/pip/issues/13154&gt;</code>_)</li>
</ul>
<h2>Features</h2>
<ul>
<li>Prefer to display :pep:<code>639</code>
<code>License-Expression</code> in <code>pip show</code> if metadata
version is at least 2.4.
(<code>[#13112](pypa/pip#13112)
&lt;https://github.com/pypa/pip/issues/13112&gt;</code>_)</li>
<li>Support :pep:<code>639</code> <code>License-Expression</code> and
<code>License-File</code> metadata fields in JSON
output. <code>pip inspect</code> and <code>pip install --report</code>
now emit
<code>license_expression</code> and <code>license_file</code> fields in
the <code>metadata</code> object,
if the corresponding fields are present in the installed
<code>METADATA</code> file.
(<code>[#13134](pypa/pip#13134)
&lt;https://github.com/pypa/pip/issues/13134&gt;</code>_)</li>
<li>Files in the network cache will inherit the read/write permissions
of pip's cache
directory (in addition to the current user retaining read/write access).
This
enables a single cache to be shared among multiple users.
(<code>[#11012](pypa/pip#11012)
&lt;https://github.com/pypa/pip/issues/11012&gt;</code>_)</li>
<li>Return the size, along with the number, of files cleared on
<code>pip cache purge</code> and <code>pip cache remove</code>
(<code>[#12176](pypa/pip#12176)
&lt;https://github.com/pypa/pip/issues/12176&gt;</code>_)</li>
<li>Cache <code>python-requires</code> checks while filtering potential
installation candidates.
(<code>[#13128](pypa/pip#13128)
&lt;https://github.com/pypa/pip/issues/13128&gt;</code>_)</li>
<li>Optimize package collection by avoiding unnecessary URL parsing and
other processing.
(<code>[#13132](pypa/pip#13132)
&lt;https://github.com/pypa/pip/issues/13132&gt;</code>_)</li>
</ul>
<h2>Bug Fixes</h2>
<ul>
<li>Reorder the encoding detection when decoding a requirements file,
relying on
UTF-8 over the locale encoding by default, matching the documented
behaviour.
(<code>[#12771](pypa/pip#12771)
&lt;https://github.com/pypa/pip/issues/12771&gt;</code>_)</li>
<li>The pip version self check is disabled on
<code>EXTERNALLY-MANAGED</code> environments.
(<code>[#11820](pypa/pip#11820)
&lt;https://github.com/pypa/pip/issues/11820&gt;</code>_)</li>
<li>Fix a security bug allowing a specially crafted wheel to execute
code during
installation. (<code>[#13079](pypa/pip#13079)
&lt;https://github.com/pypa/pip/issues/13079&gt;</code>_)</li>
<li>The inclusion of <code>packaging</code> 24.2 changes how pre-release
specifiers with <code>&lt;</code> and <code>&gt;</code>
behave. Including a pre-release version with these specifiers now
implies
accepting pre-releases (e.g., <code>&lt;2.0dev</code> can include
<code>1.0rc1</code>). To avoid
implying pre-releases, avoid specifying them (e.g., use
<code>&lt;2.0</code>).
The exception is <code>!=</code>, which never implies pre-releases.
(<code>[#13163](pypa/pip#13163)
&lt;https://github.com/pypa/pip/issues/13163&gt;</code>_)</li>
<li>The <code>--cert</code> and <code>--client-cert</code> command-line
options are now respected while
installing build dependencies. Consequently, the private
<code>_PIP_STANDALONE_CERT</code>
environment variable is no longer used.
(<code>[#5502](pypa/pip#5502)
&lt;https://github.com/pypa/pip/issues/5502&gt;</code>_)</li>
<li>The <code>--proxy</code> command-line option is now respected while
installing build dependencies.
(<code>[#6018](pypa/pip#6018)
&lt;https://github.com/pypa/pip/issues/6018&gt;</code>_)</li>
</ul>
<h2>Vendored Libraries</h2>
<ul>
<li>Upgrade CacheControl to 0.14.1</li>
<li>Upgrade idna to 3.10</li>
<li>Upgrade msgpack to 1.1.0</li>
<li>Upgrade packaging to 24.2</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/pypa/pip/commit/f47b5874299848c688336ae7c8d69534013fe2c6"><code>f47b587</code></a>
Bump for release</li>
<li><a
href="https://github.com/pypa/pip/commit/74a7f3335338712af44be95241daf62e756f27ec"><code>74a7f33</code></a>
Update AUTHORS.txt</li>
<li><a
href="https://github.com/pypa/pip/commit/a008888a5b123e8d5e4667bdd21e4b42f3fc034c"><code>a008888</code></a>
Merge pull request <a
href="https://redirect.github.com/pypa/pip/issues/13171">#13171</a> from
pypa/dependabot/github_actions/github-actions-...</li>
<li><a
href="https://github.com/pypa/pip/commit/d265fb7427c3ba4dbd10e4874a0bebea2e59350e"><code>d265fb7</code></a>
Merge pull request <a
href="https://redirect.github.com/pypa/pip/issues/13174">#13174</a> from
ichard26/changelog</li>
<li><a
href="https://github.com/pypa/pip/commit/d35384ef91cb372a5223a01f980e5deb84c8fde5"><code>d35384e</code></a>
Copyedit news entries before 25.0</li>
<li><a
href="https://github.com/pypa/pip/commit/adc4f9951b51b6a06e405b8960dd0c5f030f0fb5"><code>adc4f99</code></a>
Reorder requirements file decoding (<a
href="https://redirect.github.com/pypa/pip/issues/12795">#12795</a>)</li>
<li><a
href="https://github.com/pypa/pip/commit/40c42149a51a63e8416c047d5ddc0da1694387ea"><code>40c4214</code></a>
Bump pypa/gh-action-pypi-publish in the github-actions group</li>
<li><a
href="https://github.com/pypa/pip/commit/6b0fb904803fbb3ce7da63966b2759407b6cd9dc"><code>6b0fb90</code></a>
Merge pull request <a
href="https://redirect.github.com/pypa/pip/issues/13048">#13048</a> from
sbidoul/trusted-publisher-sbi</li>
<li><a
href="https://github.com/pypa/pip/commit/c7fb1e13ec79b1b48481ac245144c2b368e64f7d"><code>c7fb1e1</code></a>
Merge pull request <a
href="https://redirect.github.com/pypa/pip/issues/13145">#13145</a> from
befeleme/pip-show-pep639</li>
<li><a
href="https://github.com/pypa/pip/commit/41c807c5938d269703c6ff2644fb3b7dc88eda4e"><code>41c807c</code></a>
Show License-Expression if present in package metadata</li>
<li>Additional commits viewable in <a
href="https://github.com/pypa/pip/compare/24.3.1...25.0">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pip&package-manager=pip&previous-version=24.3.1&new-version=25.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants