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 2020/12/05 03:28:49 UTC
[GitHub] [airflow] htgeis opened a new pull request #12825: Enable to expand the key file path in ssh hooks
htgeis opened a new pull request #12825:
URL: https://github.com/apache/airflow/pull/12825
Sometimes, the key_file path users defined in the ssh connection is not a absolute path but a path combined with shell variable e.g. `${AIRFLOW_HOME}/**/**.rsa` or `${SECRET_FOLDER}/**/**.rsa.`
These variables maybe change in the future and it's better not to do hard code in the connection.
So I think it should be useful to support expanding the key file path in ssh hook.
----------------------------------------------------------------
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
[GitHub] [airflow] ashb commented on pull request #12825: Enable to expand the key file path in ssh hooks
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #12825:
URL: https://github.com/apache/airflow/pull/12825#issuecomment-756058988
Use a "Connection" for this instead -- that way your dag authors don't need to know _anything_ other than the connection id.
----------------------------------------------------------------
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
[GitHub] [airflow] htgeis commented on pull request #12825: Enable to expand the key file path in ssh hooks
Posted by GitBox <gi...@apache.org>.
htgeis commented on pull request #12825:
URL: https://github.com/apache/airflow/pull/12825#issuecomment-739450965
> > These variables maybe change in the future and it's better not to do hard code in the connection.
>
> This doesn't convince me: anything can change, including the non-envvar parts. File paths here should be _explicit_ and DAG author should have full responsibility for that.
In our case which is a multi-tenant scenario, the dag authors could know the relative path of their key files but the root path we don't want to let them care about it. It did provide more flexibility for us and `expandvars` is just a simple logic which I think would not have impact for the original user cases.
----------------------------------------------------------------
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
[GitHub] [airflow] XD-DENG commented on pull request #12825: Enable to expand the key file path in ssh hooks
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on pull request #12825:
URL: https://github.com/apache/airflow/pull/12825#issuecomment-739407314
> These variables maybe change in the future and it's better not to do hard code in the connection.
This doesn't convince me: anything can change, including the non-envvar parts. File paths here should be _explicit_ and DAG author should have full responsibility for that.
----------------------------------------------------------------
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
[GitHub] [airflow] ashb closed pull request #12825: Enable to expand the key file path in ssh hooks
Posted by GitBox <gi...@apache.org>.
ashb closed pull request #12825:
URL: https://github.com/apache/airflow/pull/12825
----------------------------------------------------------------
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
[GitHub] [airflow] XD-DENG commented on pull request #12825: Enable to expand the key file path in ssh hooks
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on pull request #12825:
URL: https://github.com/apache/airflow/pull/12825#issuecomment-739408758
It's good to have flexibility only to certain extend. Being too flexible may lead to unexpected confusion eventually.
I look forward to opinions from other committers as well. But for me personally I prefer not to make this change.
----------------------------------------------------------------
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
[GitHub] [airflow] XD-DENG edited a comment on pull request #12825: Enable to expand the key file path in ssh hooks
Posted by GitBox <gi...@apache.org>.
XD-DENG edited a comment on pull request #12825:
URL: https://github.com/apache/airflow/pull/12825#issuecomment-739408758
It's good to have flexibility only to certain extent. Being too flexible may lead to unexpected confusion eventually.
I look forward to opinions from other committers as well. But for me personally I prefer not to make this change.
----------------------------------------------------------------
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