Add response hook #2605

dadocsis · 2017-12-12T22:44:15Z

Are there changes in behavior for the user?

None

Related issue number

#434

Checklist

I think the code is well written
[x ] Unit tests for the changes exist
[ x] Documentation reflects the changes
[ x] 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.
[ x] Add a new news fragment into the CHANGES folder
- name it <issue_id>.<type> for example (588.bug)
- 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."

- retrofit BasicAuth

codecov-io · 2017-12-12T23:10:47Z

Codecov Report

Merging #2605 into master will increase coverage by 0.02%.
The diff coverage is 98.24%.

@@            Coverage Diff            @@
##           master   #2605      +/-   ##
=========================================
+ Coverage   97.87%   97.9%   +0.02%     
=========================================
  Files          38      38              
  Lines        7304    7355      +51     
  Branches     1263    1278      +15     
=========================================
+ Hits         7149    7201      +52     
+ Misses         51      50       -1     
  Partials      104     104

Impacted Files	Coverage Δ
aiohttp/web_request.py	`99.68% <ø> (ø)`	⬆️
aiohttp/http_writer.py	`99.42% <ø> (+1.07%)`	⬆️
aiohttp/web_runner.py	`100% <ø> (ø)`
aiohttp/web_response.py	`98.18% <ø> (-0.08%)`	⬇️
aiohttp/test_utils.py	`98.57% <100%> (+0.01%)`	⬆️
aiohttp/web.py	`98.76% <100%> (ø)`	⬆️
aiohttp/web_app.py	`98.81% <100%> (ø)`	⬆️
aiohttp/payload.py	`98.74% <100%> (+0.02%)`	⬆️
aiohttp/web_fileresponse.py	`97.88% <100%> (-0.05%)`	⬇️
aiohttp/web_protocol.py	`89.42% <100%> (-0.11%)`	⬇️
... and 5 more

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 94e6ee5...d7df00a. Read the comment docs.

asvetlov · 2017-12-13T14:18:54Z

I dislike the proposal by 2 reasons:

Named hooks are too subtle. I understand, requests uses this approach but if we need a hook for calling a code just after retrieving response header -- let's add just it without inviting very generic and potentially error-prone approach.
The PR calls auth helper passing ClientRequest instance. ClientRequest is not designed to be a part of public API, it is not stable and changed very often. I don't like the class design but can live with it unless we don't expose the class.

hellysmile · 2017-12-13T15:03:26Z

It looks like solution for implementing aio-libs/aioelasticsearch#48 , requests has https://pypi.python.org/pypi/requests-aws4auth , right now it is very tricky to implement something like requests-aws4auth for aiohttp

dadocsis · 2017-12-13T15:28:02Z

@asvetlov
Just want to clarify
#1. So just create a single static hook?
#2. I see what you are saying about it not being apart of the api. Unfortunately non trivial auth schemes will need the original request object. Do you have an actionable suggestion for me?

* Drop encoding param * Add CHANGES

asvetlov · 2017-12-13T15:46:21Z

Please give me a pause.
I'll try to cook something.

I understand the need and share the intention, but maybe I can invent an alternative implementation.

* Support .netrc by trust_env

* Avoid to create unnecessary resources (aio-libs#2586) Do not create a new resource when adding a route with the same name and path of the last added resource. * Add a test for AbstractResource().raw_match() * PrefixResource().raw_match() should always return False

…s#2611) * Add support to Flask-style decorators with class-based Views * Add versionadded 3.0 in documentation

…bs#2612) Before this commit, the TCP tunning was allowed from PayloadWriter and Response classes, this was exclusively used by just some edge cases and caused some overhead in performance and maintainability. Since now, the TCP tunning has to be done using the stream and its the responsibility of the developer set and set back the proper values.

* allow custom port to TestServer * tweaks and add CHANGES * add docs * add docs for pytest fixture unused_port * addinv versionadded:: 3.0

* Function web.run_app add param access_log_class * Docs of web.run_app add param access_log_class * Add changelog and contributor * make isort * Update docs/web_reference.rst

- retrofit BasicAuth

… into add_response_hook

Cellebyte · 2019-01-29T12:46:26Z

Will this be merged? I need this to implement some custom auth handlers for OpenID token-exchange client Authorization and Authentication in my async client implementation.

asvetlov · 2019-01-29T12:53:02Z

I understand your needs but the functionality should be implemented in a slightly different way than proposed by this PR.

Cellebyte · 2019-01-29T13:20:53Z

How do I implement it now? Should I use decorators?

asvetlov · 2019-01-29T13:35:10Z

Now there is the inconvenient and undocumented way with passing request_class to ClientSession constructor.
The functionality will be changed/removed in aiohttp 3.5, sorry -- after implementing the proper solution.

Cellebyte · 2019-01-29T14:19:06Z

OK, so i do it the messy way. Thanks

asvetlov · 2019-01-29T14:25:39Z

Sorry about that.
I really want to improve things, it is on my todo list.
The solution touches several aspects, it is not 20-lines change.
I hope to introduce something in a month or two.

Al added 5 commits December 5, 2017 14:42

code and unit tests

e47676b

per flake8 recomendations

312e72b

- Update docs

083172d

- retrofit BasicAuth

Merge branch 'upstream/master' into add_response_hook

a6ab91d

- add myself to contributors.txt

a29dc9c

Al added 2 commits December 13, 2017 09:10

increase coverage

b988674

Merge branch 'upstream/master' into add_response_hook

ab5dc4d

Drop encoding param (aio-libs#2606)

7e3f555

* Drop encoding param * Add CHANGES

asvetlov and others added 17 commits December 14, 2017 07:47

Fix docstrings for req.host and req.remote (aio-libs#2607)

7054639

Merge branch '2.3'

affbbd9

Update spelling whitelist

245d3bf

Merge branch '2.3'

f0fe923

Support .netrc by trust_env (aio-libs#2584)

76f9274

* Support .netrc by trust_env

Document .netrc support

dd234f3

Fix small typos (aio-libs#2609)

c256c29

Fix documentation around JSON and content-type (aio-libs#2598)

29e5eac

Small documentation fix

1cb4fa2

Fix docs typo

d02ca2a

Add Nikolay Kim to contributors

05a5a6c

Add support to Flask-style decorators with class-based Views (aio-lib…

c178a99

…s#2611) * Add support to Flask-style decorators with class-based Views * Add versionadded 3.0 in documentation

Fix aio-libs#2604: Drop tcp_cork/tcp_nodelay from docs

4412fc9

allow custom port to TestServer (aio-libs#2613)

9cc03cd

* allow custom port to TestServer * tweaks and add CHANGES * add docs * add docs for pytest fixture unused_port * addinv versionadded:: 3.0

web_site -> web_runner

bd1c617

asvetlov and others added 16 commits December 19, 2017 12:13

Add a link to aiohttp-remotes

51961a7

Merge branch '2.3'

6f955fb

Fix spelling

c4efde8

Merge branch '2.3'

97b9373

web.run_app support access_log_class param (aio-libs#2616)

19c138c

* Function web.run_app add param access_log_class * Docs of web.run_app add param access_log_class * Add changelog and contributor * make isort * Update docs/web_reference.rst

Add source=self for ResourceWarning

b513a1e

Fix tests

dc7d0c5

Restore compatibility with Python 3.5

6650b35

fix test case

d45a1ee

code and unit tests

3406c86

per flake8 recomendations

9a2955f

- Update docs

7b9e56f

- retrofit BasicAuth

- add myself to contributors.txt

7f7ede0

increase coverage

9fde198

fix test case

f1fec44

Merge branch 'add_response_hook' of https://github.com/dadocsis/aiohttp…

d7df00a

… into add_response_hook

asvetlov closed this Jan 29, 2019

lock bot added the outdated label Jan 30, 2020

lock bot locked as resolved and limited conversation to collaborators Jan 30, 2020

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Jan 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add response hook #2605

Add response hook #2605

dadocsis commented Dec 12, 2017 •

edited by asvetlov

Loading

codecov-io commented Dec 12, 2017 •

edited

Loading

asvetlov commented Dec 13, 2017

hellysmile commented Dec 13, 2017

dadocsis commented Dec 13, 2017

asvetlov commented Dec 13, 2017

Cellebyte commented Jan 29, 2019

asvetlov commented Jan 29, 2019

Cellebyte commented Jan 29, 2019

asvetlov commented Jan 29, 2019

Cellebyte commented Jan 29, 2019

asvetlov commented Jan 29, 2019

Add response hook #2605

Add response hook #2605

Conversation

dadocsis commented Dec 12, 2017 • edited by asvetlov Loading

Are there changes in behavior for the user?

Related issue number

Checklist

codecov-io commented Dec 12, 2017 • edited Loading

Codecov Report

asvetlov commented Dec 13, 2017

hellysmile commented Dec 13, 2017

dadocsis commented Dec 13, 2017

asvetlov commented Dec 13, 2017

Cellebyte commented Jan 29, 2019

asvetlov commented Jan 29, 2019

Cellebyte commented Jan 29, 2019

asvetlov commented Jan 29, 2019

Cellebyte commented Jan 29, 2019

asvetlov commented Jan 29, 2019

dadocsis commented Dec 12, 2017 •

edited by asvetlov

Loading

codecov-io commented Dec 12, 2017 •

edited

Loading