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.

Sign up for GitHub

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

Jump to bottom

Added asgi version and spec_version to scope #597

Merged
merged 10 commits into from
May 4, 2020
Merged

Added asgi version and spec_version to scope #597

merged 10 commits into from
May 4, 2020

Conversation

euri10
Copy link
Member

@euri10 euri10 commented Mar 21, 2020 •
edited by florimondmanca
Loading

attempt at solving #578

Fixes #578

@euri10
Added asgi version and spec_version to scope
9316341 
Minor import sort corrected in a test
florimondmanca
florimondmanca reviewed Apr 10, 2020
View reviewed changes
Copy link
Member

@florimondmanca florimondmanca left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start! Did a bit of digging around scope["asgi"]["version"], my notes below.

uvicorn/protocols/http/h11_impl.py Outdated Show resolved Hide resolved
@euri10
Copy link
Member Author

euri10 commented Apr 10, 2020

@florimondmanca should I write a test for it @florimondmanca ? seems a little bit useless to me but if you want

@florimondmanca
Copy link
Member

Yes ideally let's add a test to prevent regressions. Surely there's somewhere we test the content of scope?

florimondmanca
florimondmanca reviewed Apr 10, 2020
View reviewed changes
tests/test_config.py Outdated Show resolved Hide resolved
tests/test_config.py Outdated Show resolved Hide resolved
florimondmanca
florimondmanca reviewed Apr 10, 2020
View reviewed changes
Copy link
Member

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to submit this one :-)

tests/test_config.py Outdated Show resolved Hide resolved
@euri10
Copy link
Member Author

euri10 commented May 4, 2020

I put the latest master changes, which caught some small stuff with flake8 !

I think it should be ok now, would be great if you have time to take a look at it @florimondmanca

@euri10 euri10 requested a review from florimondmanca May 4, 2020 15:28
florimondmanca
florimondmanca approved these changes May 4, 2020
View reviewed changes
Copy link
Member

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, looks good!

@euri10 euri10 merged commit ae0fd31 into encode:master May 4, 2020
@euri10 euri10 deleted the 578_missing_scopes branch May 4, 2020 17:46
mpaolini added a commit to feedstock/uvicorn that referenced this pull request Jul 29, 2020
@mpaolini
Fix crash when --interface is wsgi
938f72a 
This is a regression introduced in
ae0fd31 (encode#597)
@mpaolini mpaolini mentioned this pull request Jul 29, 2020
Merged
florimondmanca pushed a commit that referenced this pull request Jul 30, 2020
@mpaolini
Fix crash when --interface is wsgi (#730)
0380c4e 
* Fix crash when --interface is wsgi

This is a regression introduced in
ae0fd31 (#597)

* Set asgi 3.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@florimondmanca florimondmanca florimondmanca approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HTTP Scope missing fields
2 participants
@euri10 @florimondmanca