You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/04/08 09:51:41 UTC

[GitHub] [airflow] uranusjr commented on a change in pull request #14642: Entrypoint support in docker operator

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:
users@infra.apache.org