Skip to content

Conversation

@RF-Tar-Railt
Copy link
Member

No description provided.

@GreyElaina GreyElaina requested a review from Copilot September 4, 2025 10:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds Hypercorn support as an additional ASGI server option alongside the existing Uvicorn support. The changes restructure the ASGI module to support multiple server implementations and fix a naming issue in the SQLAlchemy utilities.

  • Adds comprehensive Hypercorn ASGI service implementation with custom logging integration
  • Refactors ASGI module structure to support multiple server backends with graceful fallbacks
  • Updates project dependencies and configuration to accommodate the new server options

Reviewed Changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/graia/amnesia/builtins/asgi/hypercorn.py New Hypercorn ASGI service implementation with custom logger and configuration options
src/graia/amnesia/builtins/asgi/uvicorn.py Extracted Uvicorn service from init.py with enhanced configuration options
src/graia/amnesia/builtins/asgi/init.py Refactored to support multiple ASGI servers with conditional imports and getattr fallbacks
src/graia/amnesia/builtins/asgi/asgitypes.py Updated ASGIVersions TypedDict to use total=False
src/graia/amnesia/builtins/sqla/service.py Fixed import path for get_subclasses utility
src/graia/amnesia/builtins/sqla/init.py Fixed import path and corrected typo in patch_logger function name
pyproject.toml Updated dependencies structure, separated uvi and hyper extras, reorganized dev dependencies
exam_hypercorn.py Added example file demonstrating Hypercorn usage

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.



class ASGIVersions(TypedDict):
class ASGIVersions(TypedDict, total=False):
Copy link
Member

Choose a reason for hiding this comment

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

根据 [asgiref/typing.py],这里为什么要特别声明 total=False?是 uvicorn 或 hypercorn 中有任意一个的行为与此不一致吗?

Copy link
Member Author

Choose a reason for hiding this comment

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

hypercorn.typing里用了total=False

Copy link
Member

@GreyElaina GreyElaina Sep 4, 2025

Choose a reason for hiding this comment

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

hypercorn 和 uvicorn 都兼容类似的情况,但这里建议 revert 此修改使得与 asgiref 对齐。
或者,考虑直接引入 asgiref。我现在忘记我为什么不引入 asgiref 了,或许认为该实现处于类型级别,对外部库的引用没有必要?

Copy link
Member Author

Choose a reason for hiding this comment

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

即使引入了也于事无补((
(hypercorn的typing上用了个NewType)

from .middleware import DispatcherMiddleware


async def _empty_asgi_handler(scope, receive, send):
Copy link
Member

Choose a reason for hiding this comment

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

你可以添加一个 common 模块以复用如 _empty_asgi_handler 等。

Copy link
Member

Choose a reason for hiding this comment

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

此外,不敢保证 hypercorn 是否真的需要这样一个 dummy asgi handler,我希望能予以确认。

Copy link
Member Author

Choose a reason for hiding this comment

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

hypercorn 的 serve 也需要一个 app: Framework

manager = it(Launart)
manager.add_component(serv := HypercornASGIService("127.0.0.1", 5333))
serv.patch_logger()
manager.add_component(serv := UvicornASGIService("127.0.0.1", 5333, patch_logger=True))
Copy link
Member

Choose a reason for hiding this comment

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

我不理解。建议使用 VS Code 自带的 Git 变更暂存工具,可以很直观的实现按需提交。

@RF-Tar-Railt RF-Tar-Railt merged commit dcfb879 into master Sep 5, 2025
5 checks passed
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