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-5829] Get rid of the checklicence image. #6495

Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Nov 4, 2019

This change is a further step of simplifying the set of scripts
used by CI. The separate checklicence image was implemented as an
optimisation of the licence check time. The image to download was
small and could be downloaded slightly faster in CI. However that
made all the management script more complex and lead to having
separate jobs for check licence and static checks. That lead to
actually longer time of Travis jobs - because new machine had to
be spun-off for checklicence check only.

With this change, the CI image is the only one left and it is slightly
bigger (with RAT tool added) but the same image is used for all the
tests - unit tests, static checks and checklicence checks.

This also makes it easier to manage the images and decreases update
overhead on the developers using Breeze.

Make sure you have checked all 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 (not including Jira issue reference)
    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"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
    • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

@potiuk potiuk changed the title [AIRFLOW-5829] Get rid of the checklicence image. Depends on [AIRFLOW-5826] [AIRFLOW-5827] [AIRFLOW-5830]) [AIRFLOW-5829] Get rid of the checklicence image. Depends on [AIRFLOW-5826] [AIRFLOW-5827] [AIRFLOW-5830] Nov 4, 2019
@potiuk potiuk self-assigned this Nov 4, 2019
@potiuk potiuk force-pushed the get-rid-of-checklicence-image branch from a3e0b43 to 0a4e6b7 Compare November 4, 2019 09:50
@potiuk potiuk changed the title [AIRFLOW-5829] Get rid of the checklicence image. Depends on [AIRFLOW-5826] [AIRFLOW-5827] [AIRFLOW-5830] [AIRFLOW-5829] Get rid of the checklicence image. Depends on [AIRFLOW-5827] [AIRFLOW-5830] Nov 4, 2019
Dockerfile Outdated Show resolved Hide resolved
@potiuk potiuk force-pushed the get-rid-of-checklicence-image branch 2 times, most recently from 9c794ab to 3e4348b Compare November 4, 2019 20:44
@potiuk potiuk changed the title [AIRFLOW-5829] Get rid of the checklicence image. Depends on [AIRFLOW-5827] [AIRFLOW-5830] [AIRFLOW-5829] Get rid of the checklicence image. Depends on [AIRFLOW-5830] Nov 4, 2019
@potiuk potiuk force-pushed the get-rid-of-checklicence-image branch from 3e4348b to 24562ff Compare November 4, 2019 20:57
@potiuk potiuk force-pushed the get-rid-of-checklicence-image branch 2 times, most recently from c65b53b to c9a627a Compare November 5, 2019 23:30
@potiuk potiuk changed the title [AIRFLOW-5829] Get rid of the checklicence image. Depends on [AIRFLOW-5830] [AIRFLOW-5829] Get rid of the checklicence image. Nov 5, 2019
@potiuk potiuk force-pushed the get-rid-of-checklicence-image branch from c9a627a to 2f87e82 Compare November 5, 2019 23:45
@potiuk
Copy link
Member Author

potiuk commented Nov 6, 2019

This one is green. Looking forward to merging this one as well (getting rid of checklicence image).

@potiuk
Copy link
Member Author

potiuk commented Nov 6, 2019

And we have now one static check job in Travis run with single image - less overhead for starting VMs on Travis. See https://travis-ci.org/apache/airflow/builds/607930228

@potiuk
Copy link
Member Author

potiuk commented Nov 6, 2019

Note that this build lasts longer but when we merge to master the image will be pushed to Dockerhub (I will speed it up with my build server in GCP) and they will be much faster (10 minutes each).

This change is a further step of simplifying the set of scripts
used by CI. The separate checklicence image was implemented as an
optimisation of the licence check time. The image to download was
small and could be downloaded slightly faster in CI. However that
made all the management script more complex and lead to having
separate jobs for check licence and static checks. That lead to
actually longer time of Travis jobs - because new machine had to
be spun-off for checklicence check only.

With this change, the CI image is the only one left and it is slightly
bigger (with RAT tool added) but the same image is used for all the
tests - unit tests, static checks and checklicence checks.

This also makes it easier to manage the images and decreases update
overhead on the developers using Breeze.
@potiuk potiuk force-pushed the get-rid-of-checklicence-image branch from 2f87e82 to 6bd5e9b Compare November 6, 2019 09:30
@potiuk potiuk merged commit d4ff529 into apache:master Nov 6, 2019
potiuk added a commit that referenced this pull request Nov 6, 2019
This change is a further step of simplifying the set of scripts
used by CI. The separate checklicence image was implemented as an
optimisation of the licence check time. The image to download was
small and could be downloaded slightly faster in CI. However that
made all the management script more complex and lead to having
separate jobs for check licence and static checks. That lead to
actually longer time of Travis jobs - because new machine had to
be spun-off for checklicence check only.

With this change, the CI image is the only one left and it is slightly
bigger (with RAT tool added) but the same image is used for all the
tests - unit tests, static checks and checklicence checks.

This also makes it easier to manage the images and decreases update
overhead on the developers using Breeze.

(cherry picked from commit d4ff529)
potiuk added a commit that referenced this pull request Nov 12, 2019
This change is a further step of simplifying the set of scripts
used by CI. The separate checklicence image was implemented as an
optimisation of the licence check time. The image to download was
small and could be downloaded slightly faster in CI. However that
made all the management script more complex and lead to having
separate jobs for check licence and static checks. That lead to
actually longer time of Travis jobs - because new machine had to
be spun-off for checklicence check only.

With this change, the CI image is the only one left and it is slightly
bigger (with RAT tool added) but the same image is used for all the
tests - unit tests, static checks and checklicence checks.

This also makes it easier to manage the images and decreases update
overhead on the developers using Breeze.

(cherry picked from commit d4ff529)
eladkal pushed a commit to eladkal/airflow that referenced this pull request Dec 2, 2019
This change is a further step of simplifying the set of scripts
used by CI. The separate checklicence image was implemented as an
optimisation of the licence check time. The image to download was
small and could be downloaded slightly faster in CI. However that
made all the management script more complex and lead to having
separate jobs for check licence and static checks. That lead to
actually longer time of Travis jobs - because new machine had to
be spun-off for checklicence check only.

With this change, the CI image is the only one left and it is slightly
bigger (with RAT tool added) but the same image is used for all the
tests - unit tests, static checks and checklicence checks.

This also makes it easier to manage the images and decreases update
overhead on the developers using Breeze.

(cherry picked from commit d4ff529)
kaxil pushed a commit that referenced this pull request Dec 12, 2019
This change is a further step of simplifying the set of scripts
used by CI. The separate checklicence image was implemented as an
optimisation of the licence check time. The image to download was
small and could be downloaded slightly faster in CI. However that
made all the management script more complex and lead to having
separate jobs for check licence and static checks. That lead to
actually longer time of Travis jobs - because new machine had to
be spun-off for checklicence check only.

With this change, the CI image is the only one left and it is slightly
bigger (with RAT tool added) but the same image is used for all the
tests - unit tests, static checks and checklicence checks.

This also makes it easier to manage the images and decreases update
overhead on the developers using Breeze.

(cherry picked from commit d4ff529)
@potiuk potiuk added the area:production-image Production image improvements and fixes label Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:production-image Production image improvements and fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants