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 2022/02/11 13:20:59 UTC

[GitHub] [airflow] potiuk opened a new pull request #21520: Add mssql-cli to devel extra in Airflow

potiuk opened a new pull request #21520:
URL: https://github.com/apache/airflow/pull/21520


   The change #21511 added support for db shell for MSSQL. This change
   follows up with adding mssql-cli adding to [devel] extra of airlfow
   so that it is automatically installed in Airflow Breeze CI image.
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk closed pull request #21520: Add mssql-cli to devel extra in Airflow

Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #21520:
URL: https://github.com/apache/airflow/pull/21520


   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk merged pull request #21520: Add mssql-cli to devel extra in Airflow

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #21520:
URL: https://github.com/apache/airflow/pull/21520


   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk edited a comment on pull request #21520: Add mssql-cli to devel extra in Airflow

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #21520:
URL: https://github.com/apache/airflow/pull/21520#issuecomment-1037334254


   Hey @ephraimbuddy  - I found a way of adding the mssql-cli without impacting our dependencies (I am using pipx to install it).
   
   @uranusjr - re pipx - is it often the case that package scripts need some adjustments when installed by `pipx` to make it works (like in this case?) I guess because they are using own bash scripts rather than relying on entrypoint? 
   
   I had to do this to make it works:
   
   ```
       # Unfortunately mssql-cli installed by `pipx` does not work out of the box because it uses
       # its own execution bash script which is not compliant with the auto-activation of
       # pipx venvs - we need to manually patch Python executable in the script to fix it: ¯\_(ツ)_/¯
       sed "s/python /\/root\/\.local\/pipx\/venvs\/mssql-cli\/bin\/python /" -i /root/.local/bin/mssql-cli
   ```
   
   The original script was this (which had no chance to run when it was installed by pipx):
   
   ```
   #!/usr/bin/env bash
   
   SOURCE="${BASH_SOURCE[0]}"
   while [ -h "$SOURCE" ]; do # resolve $SOURCE until the file is no longer a symlink
     DIR="$( cd -P "$( dirname "$SOURCE" )" && pwd )"
     SOURCE="$(readlink "$SOURCE")"
     [[ $SOURCE != /* ]] && SOURCE="$DIR/$SOURCE" # if $SOURCE was a relative symlink, we need to resolve it relative to the path where the symlink file was located
   done
   DIR="$( cd -P "$( dirname "$SOURCE" )" && pwd )"
   
   # Set the /root/.local/pipx/venvs/mssql-cli/bin/python io encoding to UTF-8 by default if not set.
   if [ -z ${PYTHONIOENCODING+x} ]; then export PYTHONIOENCODING=UTF-8; fi
   
   export PYTHONPATH="${DIR}:${PYTHONPATH}"
   
   python -m mssqlcli.main "$@"
   ```
   
   
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #21520: Add mssql-cli to devel extra in Airflow

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21520:
URL: https://github.com/apache/airflow/pull/21520#issuecomment-1037334254


   Hey @ephraimbuddy  - I found a way of adding the mssql-cli without impacting our dependencies (I am using pipx to install it).
   
   @uranusjr - re pipx - is it often the case that package scripts need some adjustments when installed by `pipx` to make it works (like in this case?) I guess because they are using own bash scripts rather than relying on entrypoint? 
   
   I had to do this to make it works:
   
   ```
       # Unfortunately mssql-cli installed by `pipx` does not work out of the box because it uses
       # its own execution bash script which is not compliant with the auto-activation of
       # pipx venvs - we need to manually patch Python executable in the script to fix it: ¯\_(ツ)_/¯
       sed "s/python /\/root\/\.local\/pipx\/venvs\/mssql-cli\/bin\/python /" -i /root/.local/bin/mssql-cli
   ```
   
   The original script was this (which had no chance to run when it was installed by pipx):
   
   ```
   #!/usr/bin/env bash
   
   SOURCE="${BASH_SOURCE[0]}"
   while [ -h "$SOURCE" ]; do # resolve $SOURCE until the file is no longer a symlink
     DIR="$( cd -P "$( dirname "$SOURCE" )" && pwd )"
     SOURCE="$(readlink "$SOURCE")"
     [[ $SOURCE != /* ]] && SOURCE="$DIR/$SOURCE" # if $SOURCE was a relative symlink, we need to resolve it relative to the path where the symlink file was located
   done
   DIR="$( cd -P "$( dirname "$SOURCE" )" && pwd )"
   
   # Set the /root/.local/pipx/venvs/mssql-cli/bin/python io encoding to UTF-8 by default if not set.
   if [ -z ${PYTHONIOENCODING+x} ]; then export PYTHONIOENCODING=UTF-8; fi
   
   export PYTHONPATH="${DIR}:${PYTHONPATH}"
   
   /root/.local/pipx/venvs/mssql-cli/bin/python -m mssqlcli.main "$@"
   ```
   
   
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk closed pull request #21520: Add mssql-cli to devel extra in Airflow

Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #21520:
URL: https://github.com/apache/airflow/pull/21520


   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #21520: Add mssql-cli to devel extra in Airflow

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21520:
URL: https://github.com/apache/airflow/pull/21520#issuecomment-1037334254


   Hey @ephraimbuddy  - I found a way of adding the mssql-cli without impacting our dependencies (I am using pipx to install it).
   
   @uranusjr - re pipx - is it often the case that package scripts need some adjustments when installed by `pipx` to make it works (like in this case?) I guess because they are using own bash scripts rather than relying on entrypoint? 
   
   I had to do this to make it works:
   
   ```
       # Unfortunately mssql-cli installed by `pipx` does not work out of the box because it uses
       # its own execution bash script which is not compliant with the auto-activation of
       # pipx venvs - we need to manually patch Python executable in the script to fix it: ¯\_(ツ)_/¯
       sed "s/python /\/root\/\.local\/pipx\/venvs\/mssql-cli\/bin\/python /" -i /root/.local/bin/mssql-cli
   ```
   
   The original script was this (which had no chance to run when it was installed by pipx):
   
   ```
   #!/usr/bin/env bash
   
   SOURCE="${BASH_SOURCE[0]}"
   while [ -h "$SOURCE" ]; do # resolve $SOURCE until the file is no longer a symlink
     DIR="$( cd -P "$( dirname "$SOURCE" )" && pwd )"
     SOURCE="$(readlink "$SOURCE")"
     [[ $SOURCE != /* ]] && SOURCE="$DIR/$SOURCE" # if $SOURCE was a relative symlink, we need to resolve it relative to the path where the symlink file was located
   done
   DIR="$( cd -P "$( dirname "$SOURCE" )" && pwd )"
   
   # Set the /root/.local/pipx/venvs/mssql-cli/bin/python io encoding to UTF-8 by default if not set.
   if [ -z ${PYTHONIOENCODING+x} ]; then export PYTHONIOENCODING=UTF-8; fi
   
   export PYTHONPATH="${DIR}:${PYTHONPATH}"
   
   /root/.local/pipx/venvs/mssql-cli/bin/python -m mssqlcli.main "$@"
   ```
   
   
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk merged pull request #21520: Add mssql-cli to devel extra in Airflow

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #21520:
URL: https://github.com/apache/airflow/pull/21520


   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk edited a comment on pull request #21520: Add mssql-cli to devel extra in Airflow

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #21520:
URL: https://github.com/apache/airflow/pull/21520#issuecomment-1037334254


   Hey @ephraimbuddy  - I found a way of adding the mssql-cli without impacting our dependencies (I am using pipx to install it).
   
   @uranusjr - re pipx - is it often the case that package scripts need some adjustments when installed by `pipx` to make it works (like in this case?) I guess because they are using own bash scripts rather than relying on entrypoint? 
   
   I had to do this to make it works:
   
   ```
       # Unfortunately mssql-cli installed by `pipx` does not work out of the box because it uses
       # its own execution bash script which is not compliant with the auto-activation of
       # pipx venvs - we need to manually patch Python executable in the script to fix it: ¯\_(ツ)_/¯
       sed "s/python /\/root\/\.local\/pipx\/venvs\/mssql-cli\/bin\/python /" -i /root/.local/bin/mssql-cli
   ```
   
   The original script was this (which had no chance to run when it was installed by pipx):
   
   ```
   #!/usr/bin/env bash
   
   SOURCE="${BASH_SOURCE[0]}"
   while [ -h "$SOURCE" ]; do # resolve $SOURCE until the file is no longer a symlink
     DIR="$( cd -P "$( dirname "$SOURCE" )" && pwd )"
     SOURCE="$(readlink "$SOURCE")"
     [[ $SOURCE != /* ]] && SOURCE="$DIR/$SOURCE" # if $SOURCE was a relative symlink, we need to resolve it relative to the path where the symlink file was located
   done
   DIR="$( cd -P "$( dirname "$SOURCE" )" && pwd )"
   
   # Set the /root/.local/pipx/venvs/mssql-cli/bin/python io encoding to UTF-8 by default if not set.
   if [ -z ${PYTHONIOENCODING+x} ]; then export PYTHONIOENCODING=UTF-8; fi
   
   export PYTHONPATH="${DIR}:${PYTHONPATH}"
   
   python -m mssqlcli.main "$@"
   ```
   
   
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #21520: Add mssql-cli to devel extra in Airflow

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #21520:
URL: https://github.com/apache/airflow/pull/21520#issuecomment-1036232022


   The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk closed pull request #21520: Add mssql-cli to devel extra in Airflow

Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #21520:
URL: https://github.com/apache/airflow/pull/21520


   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #21520: Add mssql-cli to devel extra in Airflow

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21520:
URL: https://github.com/apache/airflow/pull/21520#issuecomment-1036330607


   Unfortunately mssql-cli depends on a very, very old sqlparse library version which makes it conflict witth apache-drill :(. 


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org