Prepare response headers prior to running prepare signal#4263
Prepare response headers prior to running prepare signal#4263Qix- wants to merge 1 commit intoaio-libs:masterfrom
Conversation
psf-chronographer
bot
added
the
bot:chronographer:provided
|
Compliments on the testing and CI setup for this repository by the way, quite exemplary work done here. It was a pleasure working with this codebase given that I've never touched it before. Quite rare nowadays. |
| return writer | ||
|
|
||
| async def _prepare_headers( | ||
| self, request: 'BaseRequest', writer: AbstractStreamWriter |
There was a problem hiding this comment.
Should it accept a writer argument given that the same thing is effectively available via self._payload_writer anyway?
There was a problem hiding this comment.
Makes sense. request is available as self._req also.
There was a problem hiding this comment.
The issue is self._payload_writer and self._req are Optional[...] so _prepare_headers and _write_headers would need to guard against them being None to pacify mypy. Should they raise errors if that happens? Passing the non-Optional values might be preferable.
| headers[hdrs.CONNECTION] = 'close' | ||
|
|
||
| async def _write_headers( | ||
| self, request: 'BaseRequest', writer: AbstractStreamWriter |
asvetlov
left a comment
asvetlov
left a comment
There was a problem hiding this comment.
Looks very good!
Please address two very minor comments.
| return writer | ||
|
|
||
| async def _prepare_headers( | ||
| self, request: 'BaseRequest', writer: AbstractStreamWriter |
There was a problem hiding this comment.
Makes sense. request is available as self._req also.
|
@Qix- ping |
|
Will this get merged anytime soon? |
|
Hi - I don't use this library anymore as I left the team that did. Someone else is free to pick it up if they'd like to. |
|
Done by #5064 |
5 participants
What do these changes do?
When a response is to be sent, this prepares the headers prior to running the prepare hook instead of after.
Are there changes in behavior for the user?
Any uses of the response prepare signal that assumed the headers are replaced after their signal handlers will now need to take this change into account.
Related issue number
Closes #1958.
Checklist
CONTRIBUTORS.txtCHANGESfolder<issue_id>.<type>for example (588.bugfix)issue_idchange it to the pr id after creating the pr.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.