Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Jun 10, 2020

Fixes #9200


Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Unit tests coverage for changes (not needed for documentation changes)
  • Target Github ISSUE in description if exists
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

@potiuk potiuk requested a review from mik-laj June 10, 2020 18:54
@potiuk
Copy link
Member Author

potiuk commented Jun 10, 2020

Please check @mik-laj if it solves your problem. It worked in my tests.

@mik-laj
Copy link
Member

mik-laj commented Jun 10, 2020

@potiuk I'm working on it right now.
My code:

if [ -t 0 ] ; then
    DOCKER_ARGS_ARGS+=(
        --tty \\
    )
fi
if [ -t 1 ] ; then
    DOCKER_ARGS_ARGS+=(
        --interactive \\
    )
fi

@potiuk
Copy link
Member Author

potiuk commented Jun 10, 2020

But I think the second part is not correct --interactive is about stdin, not stdout and has nothing to do with terminal allocation.

@mik-laj
Copy link
Member

mik-laj commented Jun 10, 2020

Hello.
I proposed a similar change. We found a solution at the same time, but I still had to solve other problems.
https://github.com/apache/airflow/pull/9219/files
Do you want to continue working on this change?

Best regards,
Kamil

@mik-laj
Copy link
Member

mik-laj commented Jun 11, 2020

Substituted by. #9223

@mik-laj mik-laj closed this Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI tool wrappers doesn't support pipe

2 participants