akki commented on issue #5489: [AIRFLOW-4843] Allow orchestration via Docker 
Swarm (SwarmOperator)
URL: https://github.com/apache/airflow/pull/5489#issuecomment-511273852
 
 
   @mik-laj 
   I see your point regarding `_execute`. I think `_execute` is actually doing 
the things a `run_image` method should be doing and hence it makes sense to 
rename it to something like `_run_image`.
   
   Regarding pull image part of the code, I think it is fairly short and 
straightforward to remain inside `execute` (I'll add a comment to it though). 
Also, this is something already there [currently in 
Airflow](https://github.com/apache/airflow/blob/18e60f4a29881a6ad6b480296f70d98c9caedbea/airflow/operators/docker_operator.py#L204)
 and not something I have introduced in this PR, so not sure if I should do 
this in this ticket. If it still makes sense to you maybe it can be done in 
another PR/commit but personally, I don't see anything wrong with it.
   
   Thanks for reviewing, much appreciated! :) 

----------------------------------------------------------------
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]


With regards,
Apache Git Services

Reply via email to