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.

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

[AIRFLOW-1669] Fix Docker import #2653

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@Fokko
Contributor

Fokko commented Oct 2, 2017

Dear Airflow maintainers,

We want to fix the Docker import. At the revert of AIRFLOW-1368 something went wrong. Currently master is not building. I've checked, and before AIRFLOW-1368 we've used docker-py (not docker) package. This package exposes the APIClient for the low-level docker communication.

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

Description

  • Here are some details about my PR, including screenshots of any UI changes:

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

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"
[AIRFLOW-1669] Fix Docker import
We want to fix the Docker import. At the revert of AIRFLOW-1368
something went wrong.
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Oct 2, 2017

Codecov Report

Merging #2653 into master will increase coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2653      +/-   ##
==========================================
+ Coverage   71.75%   71.85%   +0.09%     
==========================================
  Files         152      152              
  Lines       11749    11749              
==========================================
+ Hits         8431     8442      +11     
+ Misses       3318     3307      -11
Impacted Files Coverage Δ
airflow/operators/docker_operator.py 97.36% <100%> (ø) ⬆️
airflow/jobs.py 78.07% <0%> (+0.45%) ⬆️
airflow/utils/helpers.py 56.32% <0%> (+2.87%) ⬆️
airflow/task_runner/bash_task_runner.py 100% <0%> (+6.66%) ⬆️

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 6895483...2a03c72. Read the comment docs.

codecov-io commented Oct 2, 2017

Codecov Report

Merging #2653 into master will increase coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2653      +/-   ##
==========================================
+ Coverage   71.75%   71.85%   +0.09%     
==========================================
  Files         152      152              
  Lines       11749    11749              
==========================================
+ Hits         8431     8442      +11     
+ Misses       3318     3307      -11
Impacted Files Coverage Δ
airflow/operators/docker_operator.py 97.36% <100%> (ø) ⬆️
airflow/jobs.py 78.07% <0%> (+0.45%) ⬆️
airflow/utils/helpers.py 56.32% <0%> (+2.87%) ⬆️
airflow/task_runner/bash_task_runner.py 100% <0%> (+6.66%) ⬆️

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 6895483...2a03c72. Read the comment docs.

asfgit pushed a commit that referenced this pull request Oct 2, 2017

[AIRFLOW-1669][AIRFLOW-1368] Fix Docker import
We want to fix the Docker import. At the revert of
AIRFLOW-1368
something went wrong.

Closes #2653 from Fokko/AIRFLOW-1669-fix-docker-
import

(cherry picked from commit 938da98)
Signed-off-by: Bolke de Bruin <bolke@xs4all.nl>

@asfgit asfgit closed this in 938da98 Oct 2, 2017

asfgit pushed a commit that referenced this pull request Oct 2, 2017

[AIRFLOW-1669][AIRFLOW-1368] Fix Docker import
We want to fix the Docker import. At the revert of
AIRFLOW-1368
something went wrong.

Closes #2653 from Fokko/AIRFLOW-1669-fix-docker-
import

(cherry picked from commit 938da98)
Signed-off-by: Bolke de Bruin <bolke@xs4all.nl>
@ahvigil

This comment has been minimized.

Show comment
Hide comment
@ahvigil

ahvigil Oct 7, 2017

Contributor

FYI docker-py has been deprecated since November 2016 in favor of docker
docker/docker-py@04e9437

Contributor

ahvigil commented Oct 7, 2017

FYI docker-py has been deprecated since November 2016 in favor of docker
docker/docker-py@04e9437

@ahvigil ahvigil referenced this pull request Oct 7, 2017

Closed

[AIRFLOW-1669] fix docker import fix #2672

4 of 4 tasks complete
@Fokko

This comment has been minimized.

Show comment
Hide comment
@Fokko

Fokko Oct 7, 2017

Contributor

Thanks for mentioning. I did not work with the docker python packages, but the master branch was failing. We need to replace the docker-py with docker properly.

Contributor

Fokko commented Oct 7, 2017

Thanks for mentioning. I did not work with the docker python packages, but the master branch was failing. We need to replace the docker-py with docker properly.

mrares pushed a commit to mrares/incubator-airflow that referenced this pull request Oct 7, 2017

[AIRFLOW-1669][AIRFLOW-1368] Fix Docker import
We want to fix the Docker import. At the revert of
AIRFLOW-1368
something went wrong.

Closes apache#2653 from Fokko/AIRFLOW-1669-fix-docker-
import

mrares pushed a commit to mrares/incubator-airflow that referenced this pull request Dec 5, 2017

[AIRFLOW-1669][AIRFLOW-1368] Fix Docker import
We want to fix the Docker import. At the revert of
AIRFLOW-1368
something went wrong.

Closes apache#2653 from Fokko/AIRFLOW-1669-fix-docker-
import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment