Skip to content
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-7029] Use separate docker image for running license check #7678

Merged
merged 1 commit into from
Mar 13, 2020

Conversation

ashb
Copy link
Member

@ashb ashb commented Mar 10, 2020

Each stage of the CI tests needs to pull our ci image. By removing
java from it we can save 1-2minutes from each test stage. This is part
of that work.

See AIRFLOW-7028 for reasoning.


Issue link: AIRFLOW-7029

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

  • Description above provides context of the change
  • Commit message/PR title starts with [AIRFLOW-NNNN]. AIRFLOW-NNNN = JIRA ID*
  • Unit tests coverage for changes (not needed for documentation changes)
  • 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.

* For document-only changes commit message can start with [AIRFLOW-XXXX].


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.

@ashb
Copy link
Member Author

ashb commented Mar 10, 2020

Uses https://github.com/ashb/apache-rat-docker (which is set up to auto build). Happy to give other people maintainer rights on the repo.

@ashb ashb requested a review from mik-laj March 10, 2020 13:00
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Great!

@potiuk
Copy link
Member

potiuk commented Mar 10, 2020

Uses https://github.com/ashb/apache-rat-docker (which is set up to auto build). Happy to give other people maintainer rights on the repo.
Please add me :)

@potiuk
Copy link
Member

potiuk commented Mar 10, 2020

@potiuk
Copy link
Member

potiuk commented Mar 10, 2020

Glad we are slimming down the CI image :)

@potiuk
Copy link
Member

potiuk commented Mar 10, 2020

Plus @ashb if you wait and rebase it on top of #7656 (after we merge it) , then people using breeze will be automatically pulling & building that new image rather than rebuilding it from RAT layer - this will be much faster.

@ashb
Copy link
Member Author

ashb commented Mar 10, 2020

@potiuk I don't see what rebasing on #7656 would do? Upon first look I thought it wouldn't matter which of the two are merged first...?

@potiuk
Copy link
Member

potiuk commented Mar 10, 2020

@potiuk I don't see what rebasing on #7656 would do? Upon first look I thought it wouldn't matter which of the two are merged first...?

Actually you are right even better to merge it first!

Each stage of the CI tests needs to pull our `ci` image. By removing
java from it we can save 1-2minutes from each test stage. This is part
of that work.
@ashb ashb force-pushed the separate-image-for-license-check branch from 7744258 to c0d8969 Compare March 13, 2020 13:05
if ! docker run "${EXTRA_DOCKER_FLAGS[@]}" -t \
--user "$(id -ur):$(id -gr)" \
--rm \
ashb/apache-rat:0.13-1 \
Copy link
Member

Choose a reason for hiding this comment

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

Have you ever asked the application developers if they didn't want to start publishing an official image?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not really sure which project it's part of, it seems like a side project?

Copy link
Member

Choose a reason for hiding this comment

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

This is the official Apache project. Here is the Github repository: https://github.com/apache/creadur-rat/
I've already added Docker to Travis CI, so the Docker image is an interesting next step forward.
https://github.com/apache/creadur-rat/blob/master/.travis.yml

@ashb ashb merged commit ef71ac6 into apache:master Mar 13, 2020
potiuk pushed a commit that referenced this pull request Mar 14, 2020
…7678)

Each stage of the CI tests needs to pull our `ci` image. By removing
java from it we can save 1-2minutes from each test stage. This is part
of that work.

(cherry picked from commit ef71ac6)
@ashb ashb deleted the separate-image-for-license-check branch March 16, 2020 09:39
kaxil pushed a commit that referenced this pull request Mar 19, 2020
…7678)

Each stage of the CI tests needs to pull our `ci` image. By removing
java from it we can save 1-2minutes from each test stage. This is part
of that work.

(cherry picked from commit ef71ac6)
kaxil pushed a commit to astronomer/airflow that referenced this pull request Mar 19, 2020
…pache#7678)

Each stage of the CI tests needs to pull our `ci` image. By removing
java from it we can save 1-2minutes from each test stage. This is part
of that work.

(cherry picked from commit ef71ac6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants