Open
Conversation
- Use `Any` instead of `any` - Import `__future__.annotations` for compatibility - Use `|` operator instead of `Union[]` - Only use `Any` if `Any` is used in the union - Fix `super().__init__()` arguments - None check if required - Use `ABC` base class to `AbstractHandlerBase`, use `@abstractmethod` to the class's methods
Potato1682
added
documentation
PeyaPeyaPeyang
requested changes
Sep 26, 2021
| self.count += 1 | ||
|
|
||
| def get_endpoint(self, method: str, path: str, params: Optional[dict] = None) -> Optional[EndPoint]: | ||
| def get_endpoint(self, method: str, path: str, params: dict = {}) -> Optional[EndPoint]: |
| self.call_handler(path.path, {}, queries) | ||
| else: | ||
| if "Content-Type" in self.request.headers: | ||
| if self.request.headers is not None and "Content-Type" in self.request.headers: |
Member
There was a problem hiding this comment.
self.request.headers がNoneになることはありません。
|
|
||
|
|
||
| class AbstractHandlerBase(StreamRequestHandler): | ||
| class AbstractHandlerBase(ABC, StreamRequestHandler): |
Member
Author
There was a problem hiding this comment.
このクラスからの継承を止めて、子クラスで維持させた方がいいかもしれません。
全部の子クラスで使用されている関数があればabcを使用します。
src/endpoint.py
Outdated
| from __future__ import annotations | ||
| from typing import Union, Optional | ||
|
|
||
| from typing import Optional, Any, Literal |
| raise ParseException("MALFORMED_HEADER") | ||
|
|
||
| self._response.headers.add(*kv) | ||
| self._response.headers.add(*kv) if self._response.headers is not None else None |
Member
There was a problem hiding this comment.
self._response.headersが None になることはありません。
Member
Author
There was a problem hiding this comment.
Noneチェックが必要ない型にするには__init__からNone定義をしている箇所を削除する必要があります。
| self.multiple = True | ||
| elif req.headers["Connection"] == "close": | ||
| self.multiple = False | ||
| if req.headers is not None: |
Member
There was a problem hiding this comment.
req.headersがNoneになることはありません。
|
|
||
| def handle_switch(self): | ||
| try: | ||
| if self.request is None: |
Member
There was a problem hiding this comment.
self.request がNoneになることはありません。
| self.logger.info(get_log_name(), '%s -- %s %s -- "%s %s"' % | ||
| (kwargs["client"], kwargs["code"], "" if kwargs["message"] is None else kwargs["message"], | ||
| self.request.method, kwargs["path"])) | ||
| self.request.method if self.request is not None else "<no method>", kwargs["path"])) |
Member
There was a problem hiding this comment.
self.request がNoneになることはありません。
| ep = endpoint.loader.get_endpoint(self.request.method, path, path_param) | ||
| ep: Optional[endpoint.EndPoint] = None | ||
|
|
||
| if self.request is not None and self.request.method is not None: |
Member
There was a problem hiding this comment.
self.request やself.request.methodNoneになることはありません。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
レビューを承認する前に入念に確認してください。見落としがあるかもしれません。
anyの代わりにAnyを使用-
any()を指定していることになっています。__future__.annotationsfor compatibility- 互換性維持のため。
|operator instead ofUnion[]-
__future__.annotationsが適用されている状態でこの記法ができます。AnyifAnyis used in the union-
Anyはすべての型を通すため、UnionやOptionalである意味がありません。Anyを使用したくない場合は代わりの型をレビューでリクエストしてください。super().__init__()arguments- 一部
super().__init__()の引数が間違っていた箇所があります。-
Noneがチェックされていない箇所にNoneチェックを入れました。ABCbase class toAbstractHandlerBase, use@abstractmethodto the class's methods-
ABCが使用されていないため入れましたが、なにか理由があれば修正します。以下の推奨事項があります。修正願います。
TypedDictを使用したdictのキーの固定tuple[]の型の設定Anyが使用されている箇所の代替となる型の指定Authorizationヘッダーのチェック部分でのNoneチェックの実装