potiuk commented on pull request #21145:
URL: https://github.com/apache/airflow/pull/21145#issuecomment-1025089492


   > Do we have to use the bash commands you have used in lines 924 and 935 via 
subcommand in python? Or do we have to try a different way? Also, I don't 
understand the comment added `In GitHub Code QL, the version of docker has 
+azure suffix which we should remove`. Could you explain to me that part?
   
   Yeah. We should run the `docker commands via subprocess`. We **could** 
potentially use docker API in python, but running docker as `docker` command is 
precisely what we want to check . In this check we want to check if the user 
has:
   
   * good docker command line version and it is on the PATH
   * the version is properly configured - when you install docker on Linux you 
have to perform [additional 
steps](https://docs.docker.com/engine/install/linux-postinstall/) to make 
docker available for you user - the "permission" error is the way to check it 
(however it would be great to see if we have the message right, it could have 
changed). The way to check it is to remove yourself from docker group or run 
docker as a diferent user who is not in `docker` group
   
   The best way to check it is to actually run the commands :)
   
   >  In GitHub Code QL, the version of docker has +azure suffix which we 
should remove.
   
   The version check simpl checks the min major/minor version and in Python it 
can be written in a nicer way. The point here is that usualy the version prnted 
by Docker is like that:
   
   ```
   docker version --format '{{.Client.Version}}'
   20.10.7
   ```
   
   However on of our users who run ./breeze in Azure Code QL ( which is Azure's 
CI system) got error that the version could not be parsed, because in Azure 
Code QL, they MODIFIED the version of docker they have installed and the 
version returened was something like:
   
   ```
   docker version --format '{{.Client.Version}}'
   20.10.7+azure
   ```
   
   :scream: :scream: :scream: :scream: :scream: 
   
   So our version check must include also this case - parsing the version . 
   


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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to