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

Expose scope parameter in request object #2432

Merged
merged 15 commits into from Apr 26, 2022

Conversation

azimovMichael
Copy link
Contributor

From #2417

@azimovMichael azimovMichael requested a review from a team as a code owner April 18, 2022 14:27
sanic/request.py Outdated Show resolved Hide resolved
sanic/request.py Outdated Show resolved Hide resolved
tests/test_request.py Outdated Show resolved Hide resolved
tests/test_request.py Outdated Show resolved Hide resolved
@prryplatypus prryplatypus linked an issue Apr 18, 2022 that may be closed by this pull request
prryplatypus
prryplatypus previously approved these changes Apr 18, 2022
Copy link
Member

@prryplatypus prryplatypus left a comment

Choose a reason for hiding this comment

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

Thanks!

sanic/request.py Outdated Show resolved Hide resolved
tests/test_request.py Outdated Show resolved Hide resolved
ahopkins
ahopkins previously approved these changes Apr 24, 2022
ahopkins
ahopkins previously approved these changes Apr 24, 2022
@codecov
Copy link

codecov bot commented Apr 24, 2022

Codecov Report

Merging #2432 (1679ac8) into main (78b6723) will increase coverage by 0.020%.
The diff coverage is 100.000%.

@@              Coverage Diff              @@
##              main     #2432       +/-   ##
=============================================
+ Coverage   87.145%   87.166%   +0.020%     
=============================================
  Files           60        60               
  Lines         5049      5057        +8     
  Branches       908       909        +1     
=============================================
+ Hits          4400      4408        +8     
  Misses         477       477               
  Partials       172       172               
Impacted Files Coverage Δ
sanic/models/protocol_types.py 53.333% <100.000%> (+3.333%) ⬆️
sanic/request.py 96.327% <100.000%> (+0.063%) ⬆️

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 78b6723...1679ac8. Read the comment docs.

@ahopkins
Copy link
Member

@azimovMichael run black on the codebase. You can also just run make pretty.

ahopkins
ahopkins previously approved these changes Apr 25, 2022
@azimovMichael
Copy link
Contributor Author

@ahopkins Not sure why py38 tests are failing, they pass locally.
The typing checks on the other hand are failing because TransportProtocol doesn't have scope parameter but we actually use MockProtocol under-the-hood

@ahopkins
Copy link
Member

I can handle the typing issue. Don't worry about the failing tests. There are some tests that we scratch our head about because they are flaky on GH but run fine locally.

@azimovMichael
Copy link
Contributor Author

@ahopkins I added scope param to the TransportProtocol, if it's ok with you @ahopkins

@ahopkins ahopkins merged commit 5d683c6 into sanic-org:main Apr 26, 2022
ChihweiLHBird pushed a commit to ChihweiLHBird/sanic that referenced this pull request Jun 1, 2022
Co-authored-by: Adam Hopkins <adam@amhopkins.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose ASGI scope on Request object
3 participants