-
Notifications
You must be signed in to change notification settings - Fork 21
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
Fixes #452: fix python linter errors #481
Conversation
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.
- The tsan failure (timeout) seems new, opening Timeout in
system_tests_grpc
#483
@@ -78,7 +76,7 @@ class IoAdapter: | |||
def __init__(self, handler: Callable, address: str, aclass: str, treatment: int) -> None: | |||
... | |||
|
|||
def send(self, message: 'Message', no_echo: int, control: int) -> None: | |||
def send(self, message: 'Message', no_echo: bool = True, control: bool = False) -> 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.
What is the difference between no_echo: bool = True
and just no_echo=True
? Is the latter old school or does the latter not fix the linter error ?
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.
The bool designation is used by the linters as a hint to check the callers to ensure they are invoking the method with the proper types.
'typing' (type hints) is a new(ish) feature in python and helps static analysis - we really should use it going forward. See https://docs.python.org/3/library/typing.html?highlight=typing#module-typing
so yeah - 'no_echo=True' is old skool .... :)
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.
@kgiusti It would help if you showed what linter warnings you are actually fixing here
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.
@jiridanek ah, yes - sorry about that:
67: ************* Module skupper_router_internal.dispatch
67: /home/kgiusti/work/skupper/skupper-router/python/skupper_router_internal/dispatch.py:44:0: W0404: Reimport 'ctypes' (imported line 38) (reimported)
67: /home/kgiusti/work/skupper/skupper-router/python/skupper_router_internal/dispatch.py:44:0: C0412: Imports from package ctypes are not grouped (ungrouped-imports)
67: ************* Module skupper_router_internal.management.agent
67: /home/kgiusti/work/skupper/skupper-router/python/skupper_router_internal/management/agent.py:863:12: E1120: No value for argument 'no_echo' in method call (no-value-for-parameter)
67: /home/kgiusti/work/skupper/skupper-router/python/skupper_router_internal/management/agent.py:863:12: E1120: No value for argument 'control' in method call (no-value-for-parameter)
The previous commit to fix #452 introduced linter errors on Fedora 34. This patch fixes those errors.