-
Notifications
You must be signed in to change notification settings - Fork 55
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
feat(core): build protocol layer to make pynest framework agnostic #98
base: main
Are you sure you want to change the base?
feat(core): build protocol layer to make pynest framework agnostic #98
Conversation
""" | ||
|
||
@property | ||
def method(self) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should change type to Literal['GET' | 'POST' | 'PUT' | 'DELETE']
Or string eum, Thus allowing string manipulation and string input
... | ||
|
||
@property | ||
def url(self) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be a UrlConstraints type (pydatic)
... | ||
|
||
@property | ||
def headers(self) -> Dict[str, str]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some headers can be json by themself.
Need to think how to represent that .
Abstract representation of an HTTP response. | ||
""" | ||
|
||
def set_status_code(self, status_code: int) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
status code should be of Enum (type checker will warn from incorrect use)
def add_route( | ||
self, | ||
path: str, | ||
endpoint: Callable[..., Any], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be using ParamSpec instead of ....
for more reading url
route_definitions = collect_route_definitions(cls, route_prefix) | ||
|
||
# 5. Store routes in class attribute for later usage | ||
setattr(cls, "__pynest_routes__", route_definitions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Magic string, should create reflection class (str_enum) for all _pynest_routes magic attributes (injectable for example) and refrence it
return route_prefix | ||
|
||
|
||
def process_dependencies(cls: Type) -> None: | ||
"""Parse and set dependencies for the class.""" | ||
"""Parse and set dependencies for the class (via your DI system).""" | ||
dependencies = parse_dependencies(cls) | ||
setattr(cls, "__dependencies__", dependencies) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reflection refrence
nest/core/decorators/controller.py
Outdated
not hasattr(method_function, "__route_path__") | ||
or not method_function.__route_path__ | ||
): | ||
if not hasattr(method_function, "__route_path__") or not method_function.__route_path__: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reflection refrence
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this comment has not been addressed yet. The commit f619cbd
outdated the comment, but the changes in the diff are not related to the "reflection refrence" mentioned in the thread. The diff shows changes to various files in the project structure and implementation details, but does not address any reflection-related concerns in the specified file and line.
nest/core/protocols.py
Outdated
|
||
def add_middleware( | ||
self, | ||
middleware_cls: Any, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be Type[MiddlewareProtocol] ?
T = TypeVar("T") | ||
|
||
|
||
class RequestAttribute: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should talk , can use new to register new Param request and reuse it.
init_subclass -> for register
new -> for casting
that can be added to the application. | ||
""" | ||
|
||
def __call__(self, request: RequestProtocol, call_next: Callable) -> Any: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should use ParamSpec
""" | ||
Create and configure the FastAPI application. | ||
""" | ||
print("Creating FastAPI app") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove print
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Implements a comprehensive protocol layer to make the PyNest framework agnostic of the underlying web framework. This major architectural change introduces new protocols for web framework abstraction, implements a FastAPI adapter, refactors core components to use these protocols, and updates the PyNest factory to support multiple adapters. The changes also include significant updates to controller decorators, error handling, and CLI support, paving the way for a more flexible and extensible framework.
Modified files (4)
Latest Contributors(0)
Modified files (5)
Latest Contributors(2)
Modified files (5)
Latest Contributors(2)
Modified files (2)
Latest Contributors(2)
Modified files (3)
Latest Contributors(2)
Modified files (3)
Latest Contributors(1)
Modified files (4)
Latest Contributors(2)