Conversation
Remove obvious Python issues reported by the IDE. The whole code should be reformated with a tool like YAPF. Signed-off-by: Volker Theile <vtheile@suse.com>
dbf0586 to
9fa4196
Compare
mgfritch
left a comment
There was a problem hiding this comment.
I'm also wondering if it might be worth converting many of the py2 string formatting style to f-strings?
| def __init__( | ||
| self, | ||
| spec, # type: ServiceSpec | ||
| get_hosts_func, # type: Callable | ||
| get_daemons_func, # type: Callable[[str],List[orchestrator.DaemonDescription]] | ||
| filter_new_host=None, # type: Optional[Callable[[str],bool]] | ||
| scheduler=None, # type: Optional[BaseScheduler] | ||
| ): |
There was a problem hiding this comment.
| def __init__( | |
| self, | |
| spec, # type: ServiceSpec | |
| get_hosts_func, # type: Callable | |
| get_daemons_func, # type: Callable[[str],List[orchestrator.DaemonDescription]] | |
| filter_new_host=None, # type: Optional[Callable[[str],bool]] | |
| scheduler=None, # type: Optional[BaseScheduler] | |
| ): | |
| def __init__( | |
| self, | |
| spec: ServiceSpec, | |
| get_hosts_func: Callable, | |
| get_daemons_func: Callable[[str],List[orchestrator.DaemonDescription]], | |
| filter_new_host: Optional[Callable[[str],bool]] = None, | |
| scheduler: Optional[BaseScheduler] = None, | |
| ): |
might as well covert these to py3 style too?
There was a problem hiding this comment.
Please do that in a separate PR.
| def name_to_auth_entity(daemon_type, # type: str | ||
| daemon_id, # type: str | ||
| host = "" # type Optional[str] = "" | ||
| host='' # type Optional[str] = "" |
There was a problem hiding this comment.
convert to py3 style type checking?
There was a problem hiding this comment.
This should be done in a separate PR. There is really really much work to do with this mgr module code.
As mentioned in the PR description, this PR will only fix obvious issues. The whole code needs a complete refactoring which should happen in a separate PR. Additionally a lint infrastructure (e.g. using tox) should be established for the cephadm mgr module to prevent various Python issues in future. I'm really tempted to chase the entire source code through YAPF or a similar tool to reformat it, but that should be done by a cephadm team member. |
|
Replaced by #36348 |
Remove obvious Python issues reported by the IDE. The whole code should be reformated with a tool like YAPF.
Signed-off-by: Volker Theile vtheile@suse.com
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard backendjenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume tox