uranusjr commented on a change in pull request #14642:
URL: https://github.com/apache/airflow/pull/14642#discussion_r609522025
##########
File path: airflow/providers/docker/operators/docker.py
##########
@@ -318,18 +321,20 @@ def _get_cli(self) -> APIClient:
tls_config = self.__get_tls_config()
return APIClient(base_url=self.docker_url,
version=self.api_version, tls=tls_config)
- def get_command(self) -> Union[List[str], str]:
+ @staticmethod
+ def get_command(command: Union[str, List[str]]) -> Union[List[str], str]:
Review comment:
It feels quite weird for me that `self.get_command()` is not internally
dependent on `self.command`. Maybe it’d be a good idea to rename this to e.g.
`format_command()` or `as_commands()`.
As an alternative, maybe it’d be reasonable to have *two* functions, one do
the formatting (replacing the new `get_command()` function in this PR), and
another that calls the formatting function to format `self.command` (like the
previous `get_command()` implementation)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
