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

Defer HTTP/1.1 parsing to Hyper (h11 project) #201

Open
1 of 3 tasks
webknjaz opened this issue May 15, 2019 · 14 comments
Open
1 of 3 tasks

Defer HTTP/1.1 parsing to Hyper (h11 project) #201

webknjaz opened this issue May 15, 2019 · 14 comments
Assignees
Labels
enhancement Improvement help wanted Somebody help us, please!

Comments

@webknjaz
Copy link
Member

webknjaz commented May 15, 2019

I suggest merging our HTTP implementation with Hyper/H11: https://github.com/python-hyper/h11 / https://h11.readthedocs.io/.

This would contribute to having a well-made parser shared with others.

I've also declared the intent @ python-hyper/h11#84

I'm submitting a ...

  • 🐞 bug report
  • 🐣 feature request
  • ❓ question about the decisions made in the repository

🐞 Is your feature request related to a problem? Please describe.

Currently, CherryPy team has to maintain its own HTTP parser implementation which doesn't scale.

🐣 Describe the solution you'd like

We should contribute to one well-designed upstream parser implementation instead of maintaining our own thing. Shared responsibility is the key to having a healthy open source project :)

📋 Describe alternatives you've considered

N/A

📋 Additional context

Hyper is a bring-your-own-I/O set of principles encouraging users to rely on protocol implementations which do not hit the network. It also potentially offers testability improvements.

It'll also prevent problems like #74

@webknjaz webknjaz mentioned this issue May 15, 2019
15 tasks
@jaraco
Copy link
Member

jaraco commented May 15, 2019

Agreed this sounds like a good idea. The trick will be - can it be implemented without causing too much disruption.

@webknjaz
Copy link
Member Author

I hope yes, but it'll require extensive testing anyway and maybe it'll also require some changes on their side. But I hope that I'll be able to dedicate some time to this.

I think I've also asked @duper51 to look into this earlier...

@webknjaz webknjaz pinned this issue May 16, 2019
@ian-otto
Copy link
Contributor

I would tend to agree with this issue, we should probably move parsing logic out of this project, as it introduces complexity that doesn't need to be here.

Agreed this sounds like a good idea. The trick will be - can it be implemented without causing too much disruption.

If there is disruption, it will be on projects relying on quirks of the existing parser (which isn't the best in terms of overall RFC compliance). I'd say if it breaks something in a dependent project, the reduced complexity will make any fixes easier.

@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Jul 23, 2019
@webknjaz webknjaz added enhancement Improvement help wanted Somebody help us, please! and removed stale This thing has been ignored for too long labels Jul 23, 2019
@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Sep 21, 2019
@webknjaz webknjaz removed the stale This thing has been ignored for too long label Sep 22, 2019
@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Nov 21, 2019
@webknjaz webknjaz removed the stale This thing has been ignored for too long label Nov 21, 2019
@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Jan 20, 2020
@ian-otto
Copy link
Contributor

Reviving this, as I have some more free time to deal with it.

Quick update, I've started work on a replacement HTTPRequest class to be a relatively "drop-in" replacement for the existing one. That said, we will likely see changes in some of the attributes that are available post-request. This is due to removing some error handling related attributes since they are now handled in h11. More updates soon (mostly just wanted to remove the stale tag).

@stale stale bot removed the stale This thing has been ignored for too long label Jan 21, 2020
@webknjaz
Copy link
Member Author

webknjaz commented Jan 21, 2020

@ian-otto it'd be great if you could start a draft PR. It doesn't have to be completed but it's useful to see the progress even in the initial versions...

@ian-otto
Copy link
Contributor

Created! See #262

@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Mar 22, 2020
@webknjaz webknjaz removed the stale This thing has been ignored for too long label Mar 22, 2020
@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label May 22, 2020
@webknjaz webknjaz removed the stale This thing has been ignored for too long label May 22, 2020
@stale

This comment has been minimized.

@stale stale bot added the stale This thing has been ignored for too long label Jul 25, 2020
@webknjaz webknjaz removed the stale This thing has been ignored for too long label Jul 25, 2020
@stale
Copy link

stale bot commented Oct 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale This thing has been ignored for too long label Oct 4, 2020
@stale stale bot closed this as completed Oct 12, 2020
@webknjaz webknjaz reopened this Jan 25, 2021
@webknjaz webknjaz removed the stale This thing has been ignored for too long label Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement help wanted Somebody help us, please!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants