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

Fix native WSGI-less HTTP server support #1785

Merged
merged 3 commits into from Jun 23, 2019

Conversation

4 participants
@webknjaz
Copy link
Member

commented Jun 22, 2019

Resolves #1377

What kind of change does this PR introduce?

  • bug fix
  • feature
  • docs update
  • tests/coverage improvement
  • refactoring
  • other

What is the related issue number (starting with #)

Fixes #1377

What is the current behavior? (You can also link to an open issue here)

When trying to use a native HTTP server, HTTP requests processing fails because Cheroot's HTTP layer stores parsed request info as bytes and CherryPy relies on it being native strings (which means Unicode text under Python 3). WSGI layer does this conversion on the Cheroot side, but the native server doesn't have this.

What is the new behavior (if this is a feature change)?

200 OK

Other information:

N/A

Checklist:

  • I think the code is well written
  • I wrote good commit messages
  • I have squashed related commits together after the changes have been approved
  • Unit tests for the changes exist
  • Integration tests for the changes exist (if applicable)
  • I used the same coding conventions as the rest of the project
  • The new code doesn't generate linter offenses
  • Documentation reflects the changes
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences

webknjaz added some commits Jun 22, 2019

@codecov

This comment has been minimized.

Copy link

commented Jun 23, 2019

Codecov Report

Merging #1785 into master will decrease coverage by 0.36%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1785      +/-   ##
==========================================
- Coverage      81%   80.64%   -0.37%     
==========================================
  Files         104      104              
  Lines       13654    13250     -404     
==========================================
- Hits        11061    10685     -376     
+ Misses       2593     2565      -28
@webknjaz

This comment has been minimized.

Copy link
Member Author

commented Jun 23, 2019

@ds6 @Kodiologist @carlodri could you please test this in your environments?

@webknjaz webknjaz requested a review from jaraco Jun 23, 2019

@webknjaz webknjaz merged commit 33e6a62 into master Jun 23, 2019

3 of 8 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
test-linux-and-macos Workflow: test-linux-and-macos
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No new or fixed alerts
Details
WIP Ready for review
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details

webknjaz added a commit that referenced this pull request Jun 23, 2019

webknjaz added a commit that referenced this pull request Jun 23, 2019

@Kodiologist

This comment has been minimized.

Copy link

commented Jun 24, 2019

@webknjaz The documentation example seems to work now. Thanks!

@ds6

This comment has been minimized.

Copy link

commented Jun 25, 2019

@webknjaz I have archived my original work as it is no longer needed, but I will try this later. My module customized a lot of low-level CherryPy features so I don't know if it will work at all on the current version.

Good work on resolving 4 year old bug tho 🎉

@carlodri

This comment has been minimized.

Copy link

commented Jun 27, 2019

@webknjaz sorry for stepping in late, and thanks for your work! Honestly I lost track of this problem, if I stumble upon it again I will report back here. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.