Add type Path on web app run #6843

SamirPS · 2022-07-23T16:53:02Z

What do these changes do?

I modify the web.py file and add Union[str,pathlib.Path] for path

Are there changes in behavior for the user?

For the option of path, we can have str or pathlib.path

Related issue number

#6839

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
- The format is <Name> <Surname>.
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES folder
- name it <issue_id>.<type> for example (588.bugfix)
- if you don't have an issue_id change it to the pr id after creating the pr
- ensure type is one of the following:
  - .feature: Signifying a new feature.
  - .bugfix: Signifying a bug fix.
  - .doc: Signifying a documentation improvement.
  - .removal: Signifying a deprecation or removal of public API.
  - .misc: A ticket has been closed, but it is not of interest to users.
- Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

SamirPS · 2022-07-23T16:58:39Z

@Dreamsorcerer Here the new

aiohttp/web_runner.py

for more information, see https://pre-commit.ci

SamirPS · 2022-07-23T23:20:17Z

The previous error was due to a python library ?

Dreamsorcerer · 2022-07-23T23:25:11Z

The previous error was due to a python library ?

Mypy gets its type information from typeshed, which is just a collection of annotations, because cpython doesn't have any annotations itself.

Dreamsorcerer · 2022-07-23T23:25:56Z

So, that error will disappear when there's a new mypy release which will include the updated typeshed.

SamirPS · 2022-07-23T23:28:25Z

Oh okay thanks for the information

now we have this error : aiohttp/web_runner.py:199: error: Incompatible return value type (got "Union[str, Path]", expected "str") [return-value]

I need to change str to Union[str, Path] for the name(self) function ?

Dreamsorcerer · 2022-07-23T23:31:57Z

You had partial changes to NamedPipeSite, I was just playing with them. I've reverted the change, as it's not clear to me whether someone would actually want to use a Path object, and the function it uses is undocumented.

SamirPS · 2022-07-23T23:34:05Z

Oh okay, use Path permit to avoid doing some convertion between Path and str i thinks

SamirPS · 2022-07-23T23:36:47Z

For the doc web_reference.rst how can i says str or Path :param str path: ?

Dreamsorcerer · 2022-07-23T23:40:17Z

For the doc web_reference.rst how can i says str or Path :param str path: ?

Ah, good point. Take a look at add_static(), it mentions Path there: https://docs.aiohttp.org/en/stable/web_reference.html#aiohttp.web.UrlDispatcher.add_static

codecov · 2022-07-23T23:47:09Z

Codecov Report

Merging #6843 (148badd) into master (934db9a) will increase coverage by 71.68%.
The diff coverage is 66.66%.

@@             Coverage Diff             @@
##           master    #6843       +/-   ##
===========================================
+ Coverage   21.76%   93.44%   +71.68%     
===========================================
  Files         104      104               
  Lines       30627    30629        +2     
  Branches     3076     3076               
===========================================
+ Hits         6666    28622    +21956     
+ Misses      23849     1838    -22011     
- Partials      112      169       +57

Flag	Coverage Δ
unit	`93.36% <66.66%> (+71.62%)`	⬆️

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

Impacted Files	Coverage Δ
tests/test_run_app.py	`86.64% <0.00%> (+62.61%)`	⬆️
aiohttp/web.py	`99.17% <100.00%> (+70.84%)`	⬆️
aiohttp/web_runner.py	`92.37% <100.00%> (+52.73%)`	⬆️
tests/autobahn/test_autobahn.py	`38.33% <0.00%> (+6.66%)`	⬆️
tests/test_circular_imports.py	`100.00% <0.00%> (+7.69%)`	⬆️
aiohttp/abc.py	`100.00% <0.00%> (+7.86%)`	⬆️
aiohttp/web_exceptions.py	`99.52% <0.00%> (+22.27%)`	⬆️
tests/conftest.py	`68.42% <0.00%> (+25.43%)`	⬆️
aiohttp/resolver.py	`64.00% <0.00%> (+28.00%)`	⬆️
aiohttp/tracing.py	`100.00% <0.00%> (+36.12%)`	⬆️
... and 87 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

SamirPS · 2022-07-23T23:48:48Z

For the doc web_reference.rst how can i says str or Path :param str path: ?

Ah, good point. Take a look at add_static(), it mentions Path there: https://docs.aiohttp.org/en/stable/web_reference.html#aiohttp.web.UrlDispatcher.add_static

I just commit the change, i hope all test will pass

SamirPS · 2022-07-24T11:53:35Z

The test which fail it's because of a gcc error

patchback · 2022-07-24T11:58:21Z

Backport to 3.9: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply f90cb19 on top of patchback/backports/3.9/f90cb1952c272936845b93a8f424b32e703d545c/pr-6843

Backporting merged PR #6843 into master

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/aio-libs/aiohttp.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.9/f90cb1952c272936845b93a8f424b32e703d545c/pr-6843 upstream/3.9

Now, cherry-pick PR Add type Path on web app run #6843 contents into that branch:
```
$ git cherry-pick -x f90cb1952c272936845b93a8f424b32e703d545c
```
If it'll yell at you with something like fatal: Commit f90cb1952c272936845b93a8f424b32e703d545c is a merge but no -m option was given., add -m 1 as follows intead:
```
$ git cherry-pick -m1 -x f90cb1952c272936845b93a8f424b32e703d545c
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Add type Path on web app run #6843 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.9/f90cb1952c272936845b93a8f424b32e703d545c/pr-6843

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Dreamsorcerer · 2022-07-24T11:59:10Z

If you could follow the steps above to get this in to 3.9, that'd be great.

SamirPS · 2022-07-24T13:01:34Z

I just do it @Dreamsorcerer and now i wait

webknjaz · 2022-08-08T14:28:44Z

aiohttp/web.py

@@ -290,7 +291,7 @@ async def _run_app(
    *,
    host: Optional[Union[str, HostSequence]] = None,
    port: Optional[int] = None,
-    path: Optional[str] = None,
+    path: Optional[Union[str, Path]] = None,


@Dreamsorcerer I wonder if this should've been os.PathLike instead...

Hmm, seems we even have a PathLike in typedefs.py.

i didn't see it, you want me to use it or keep path from Pathlib ?

I'm already on it. Updating a couple of other places too.
#6876

You update this pull request ? #6874

You update this pull request ? #6874

No, backports must be as close to the original patch as possible, for traceability.

(cherry picked from commit f90cb19)

webknjaz · 2022-09-22T00:20:30Z

3.9 backport PR: #6957.

Co-authored-by: SamirAk <[email protected]> Co-authored-by: Samir Akarioh <[email protected]>

SamirPS added 3 commits July 23, 2022 18:26

create branch and commit

4a59f3c

add optional and create two file

3b1e315

chnage some str

093208b

SamirPS requested review from webknjaz and asvetlov as code owners July 23, 2022 16:53

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Jul 23, 2022

SamirPS added 2 commits July 23, 2022 19:16

Delete str

96aeca9

add str

267a0b4

Dreamsorcerer reviewed Jul 23, 2022

View reviewed changes

aiohttp/web_runner.py Outdated Show resolved Hide resolved

SamirPS and others added 3 commits July 24, 2022 00:52

delete the str

d52eab1

Update web_runner.py

e888927

[pre-commit.ci] auto fixes from pre-commit.com hooks

609e7fb

for more information, see https://pre-commit.ci

Update web_runner.py

83028f0

Revert NamedPipeSite

84b65f3

Dreamsorcerer added the backport-3.9 label Jul 23, 2022

Update 6839.feature

4bcd6ae

change documentation for run_app

65a5f99

SamirPS requested a review from Dreamsorcerer July 23, 2022 23:59

Update web_reference.rst

148badd

Dreamsorcerer merged commit f90cb19 into aio-libs:master Jul 24, 2022

webknjaz reviewed Aug 8, 2022

View reviewed changes

webknjaz mentioned this pull request Aug 8, 2022

[PR #6843/f90cb195 backport][3.9] Add type Path on web app run #6874

Closed

5 tasks

Dreamsorcerer mentioned this pull request Aug 8, 2022

web.run_app(path=) should accept pathlib.Path #6839

Closed

webknjaz pushed a commit to webknjaz/aiohttp that referenced this pull request Sep 22, 2022

Add Path support on web app run (aio-libs#6843)

7af6165

(cherry picked from commit f90cb19)

webknjaz mentioned this pull request Sep 22, 2022

[PR #6843/f90cb195 backport][3.9] Add type Path on web app run #6957

Merged

5 tasks

webknjaz added a commit that referenced this pull request Sep 22, 2022

[PR #6843/f90cb195 backport][3.9] Add type Path on web app run (#6957)

e9e3a62

Co-authored-by: SamirAk <[email protected]> Co-authored-by: Samir Akarioh <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add type Path on web app run #6843

Add type Path on web app run #6843

SamirPS commented Jul 23, 2022 •

edited

Loading

SamirPS commented Jul 23, 2022

SamirPS commented Jul 23, 2022

Dreamsorcerer commented Jul 23, 2022

Dreamsorcerer commented Jul 23, 2022

SamirPS commented Jul 23, 2022

Dreamsorcerer commented Jul 23, 2022

SamirPS commented Jul 23, 2022

SamirPS commented Jul 23, 2022

Dreamsorcerer commented Jul 23, 2022

codecov bot commented Jul 23, 2022 •

edited

Loading

SamirPS commented Jul 23, 2022

SamirPS commented Jul 24, 2022

patchback bot commented Jul 24, 2022 •

edited

Loading

Dreamsorcerer commented Jul 24, 2022

SamirPS commented Jul 24, 2022

webknjaz Aug 8, 2022

Dreamsorcerer Aug 8, 2022

SamirPS Aug 8, 2022

Dreamsorcerer Aug 8, 2022

SamirPS Aug 8, 2022

webknjaz Aug 8, 2022

webknjaz commented Sep 22, 2022

Add type Path on web app run #6843

Add type Path on web app run #6843

Conversation

SamirPS commented Jul 23, 2022 • edited Loading

What do these changes do?

Are there changes in behavior for the user?

Related issue number

Checklist

SamirPS commented Jul 23, 2022

SamirPS commented Jul 23, 2022

Dreamsorcerer commented Jul 23, 2022

Dreamsorcerer commented Jul 23, 2022

SamirPS commented Jul 23, 2022

Dreamsorcerer commented Jul 23, 2022

SamirPS commented Jul 23, 2022

SamirPS commented Jul 23, 2022

Dreamsorcerer commented Jul 23, 2022

codecov bot commented Jul 23, 2022 • edited Loading

Codecov Report

SamirPS commented Jul 23, 2022

SamirPS commented Jul 24, 2022

patchback bot commented Jul 24, 2022 • edited Loading

Backport to 3.9: 💔 cherry-picking failed — conflicts found

Backporting merged PR #6843 into master

Dreamsorcerer commented Jul 24, 2022

SamirPS commented Jul 24, 2022

webknjaz Aug 8, 2022

Choose a reason for hiding this comment

Dreamsorcerer Aug 8, 2022

Choose a reason for hiding this comment

SamirPS Aug 8, 2022

Choose a reason for hiding this comment

Dreamsorcerer Aug 8, 2022

Choose a reason for hiding this comment

SamirPS Aug 8, 2022

Choose a reason for hiding this comment

webknjaz Aug 8, 2022

Choose a reason for hiding this comment

webknjaz commented Sep 22, 2022

SamirPS commented Jul 23, 2022 •

edited

Loading

codecov bot commented Jul 23, 2022 •

edited

Loading

patchback bot commented Jul 24, 2022 •

edited

Loading