Skip to content

[AIRFLOW-2670] Update SSH Operator to respect timeout #3553

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
Noremac201 wants to merge 1 commit into from
Closed

[AIRFLOW-2670] Update SSH Operator to respect timeout #3553

Noremac201 wants to merge 1 commit into from

Conversation

@Noremac201
Copy link
Contributor

@Noremac201 Noremac201 commented Jun 27, 2018

Make sure you have checked all steps below.

JIRA

Description

  • Here are some details about my PR, including screenshots of any UI changes:
    Previously the SSH operator was not respecting the passed in timeout to the operator. This changes that along with updating some logic inside to make it clearer what is happening based on passed in arguments.

Commits

  • My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Code Quality

  • Passes git diff upstream/master -u -- "*.py" | flake8 --diff

Fokko
Fokko reviewed Jun 29, 2018
View reviewed changes
airflow/contrib/operators/ssh_operator.py Outdated
Copy link
Contributor

@Fokko Fokko Jun 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break backward compatibility :'(

@codecov-io
Copy link

codecov-io commented Jul 2, 2018

Codecov Report

Merging #3553 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3553   +/-   ##
=======================================
  Coverage   76.83%   76.83%           
=======================================
  Files         204      204           
  Lines       15485    15485           
=======================================
  Hits        11898    11898           
  Misses       3587     3587

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 985a433...34a4e4d. Read the comment docs.

@Noremac201
Copy link
Contributor Author

Noremac201 commented Jul 6, 2018

You're right, I've reverted that back.

@Noremac201
Copy link
Contributor Author

Noremac201 commented Jul 25, 2018

Fixed by #3473

@Noremac201 Noremac201 closed this Jul 25, 2018
@Noremac201 Noremac201 mentioned this pull request Jul 30, 2018
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fokko Fokko Fokko left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Noremac201 @codecov-io @Fokko