Skip to content

Migrated to Asphalt 5#68

Open
agronholm wants to merge 18 commits into
masterfrom
asphalt5
Open

Migrated to Asphalt 5#68
agronholm wants to merge 18 commits into
masterfrom
asphalt5

Conversation

@agronholm

Copy link
Copy Markdown
Member

No description provided.

@coveralls

coveralls commented Dec 30, 2024

Copy link
Copy Markdown

Coverage Status

coverage: 99.617% (+7.3%) from 92.283%
when pulling ec37026 on asphalt5
into 917211b on master.

async with Context() as ctx:
ctx.add_resource(request, types=[Request])
async with Context():
add_resource(request, types=[Request])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
add_resource(request, types=[Request])
add_resource(request, types=Request)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

types accepts a single type, which is the case here, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, this is what it says in the docstring:

:param types: type(s) to register the resource as (omit to use the type of value)

And the annotation is:

types: type | Sequence[type] = (),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

hmm so it seems that it can accept a single type, or am I missing something?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It can, althought the naming of the parameter suggests that it should be passed an iterable of types.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fair enough, it doesn't matter a lot.

Comment thread src/asphalt/web/asgi3.py
async with Context():
scope_type = HTTPScope if scope["type"] == "http" else WebSocketScope
ctx.add_resource(scope, types=[scope_type])
add_resource(scope, types=[scope_type])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
add_resource(scope, types=[scope_type])
add_resource(scope, types=scope_type)

Comment thread src/asphalt/web/django.py
add_resource(request)
if isinstance(request, ASGIRequest):
ctx.add_resource(request.scope, types=[HTTPScope])
add_resource(request.scope, types=[HTTPScope])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
add_resource(request.scope, types=[HTTPScope])
add_resource(request.scope, types=HTTPScope)

if scope["type"] == "http":
ctx.add_resource(scope, types=[HTTPScope])
ctx.add_resource(Request(scope))
add_resource(scope, types=[HTTPScope])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
add_resource(scope, types=[HTTPScope])
add_resource(scope, types=HTTPScope)

elif scope["type"] == "websocket":
ctx.add_resource(scope, types=[WebSocketScope])
ctx.add_resource(Request(scope))
add_resource(scope, types=[WebSocketScope])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
add_resource(scope, types=[WebSocketScope])
add_resource(scope, types=WebSocketScope)

async with Context():
if scope["type"] == "http":
ctx.add_resource(scope, types=[HTTPScope])
add_resource(scope, types=[HTTPScope])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
add_resource(scope, types=[HTTPScope])
add_resource(scope, types=HTTPScope)

add_resource(scope, types=[HTTPScope])
elif scope["type"] == "websocket":
ctx.add_resource(scope, types=[WebSocketScope])
add_resource(scope, types=[WebSocketScope])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
add_resource(scope, types=[WebSocketScope])
add_resource(scope, types=WebSocketScope)


async def dispatch(self, request: Request, call_next: RequestResponseEndpoint) -> Response:
current_context().add_resource(request, types=[Request])
add_resource(request, types=[Request])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
add_resource(request, types=[Request])
add_resource(request, types=Request)

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.

3 participants