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-6662] install dumb init #7300

Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jan 30, 2020


Issue link: AIRFLOW-6662

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.

@potiuk potiuk changed the title Airflow 6662 install dumb init [AIRFLOW-6662] install dumb init Jan 30, 2020


# pylint: disable=missing-docstring
def print_stuff():
Copy link
Member

Choose a reason for hiding this comment

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

Why we need it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Used in kubernetes tests.

@potiuk potiuk force-pushed the AIRFLOW-6662-install-dumb-init branch from 9ba64ef to 5ea7f2f Compare January 31, 2020 00:33
@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Jan 31, 2020
@potiuk potiuk force-pushed the AIRFLOW-6662-install-dumb-init branch from 5ea7f2f to 2aed3a0 Compare January 31, 2020 00:48
@potiuk potiuk force-pushed the AIRFLOW-6662-install-dumb-init branch from 2aed3a0 to 58e160a Compare January 31, 2020 06:27
@ashb
Copy link
Member

ashb commented Jan 31, 2020

Though perhaps instead of a new commit we could/should just revert the previous change?

@potiuk
Copy link
Member Author

potiuk commented Jan 31, 2020

Though perhaps instead of a new commit we could/should just revert the previous change?

Let's see. I think I might not even want to revert it finally because the stability problems seems to be not related to the --init flag usage, I want to fix it first and see how we approach dumb-init eventually.

@ashb
Copy link
Member

ashb commented Jan 31, 2020

@potiuk I don't think the revert vs this would make a difference, but this still seems to be failing it's tests?

@potiuk
Copy link
Member Author

potiuk commented Jan 31, 2020

The failures are different reason - not init i am looking into it.

@potiuk
Copy link
Member Author

potiuk commented Jan 31, 2020

we have still failing master even if I fixed the "contrib/example" problem (see slack). One is print_diff import and the other is very mysterious hang that cannot be reproduced locally. Both unrelated with the init - so init was not an issue in the first place.

@potiuk potiuk force-pushed the AIRFLOW-6662-install-dumb-init branch from 58e160a to e05b47e Compare February 1, 2020 13:11
Also curl options are now using long format and include --fail
to protect against some temporary errors (5xx). Also RAT download
uses now two possible sources of downloads and fallbacks to the
second if first is not available.
@potiuk potiuk force-pushed the AIRFLOW-6662-install-dumb-init branch from e05b47e to f743d6c Compare February 2, 2020 07:35
We had stability problems with tests with --init flag so we are
going back to it
@potiuk potiuk force-pushed the AIRFLOW-6662-install-dumb-init branch from f743d6c to 2440252 Compare February 2, 2020 07:41
@codecov-io
Copy link

codecov-io commented Feb 2, 2020

Codecov Report

Merging #7300 into master will increase coverage by 0.24%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7300      +/-   ##
==========================================
+ Coverage   85.87%   86.12%   +0.24%     
==========================================
  Files         863      863              
  Lines       40520    40908     +388     
==========================================
+ Hits        34798    35232     +434     
+ Misses       5722     5676      -46
Impacted Files Coverage Δ
airflow/utils/dag_processing.py 87.93% <0%> (-0.2%) ⬇️
airflow/jobs/scheduler_job.py 89.34% <0%> (+0.14%) ⬆️
airflow/models/taskinstance.py 94.97% <0%> (+0.25%) ⬆️
airflow/providers/amazon/aws/operators/datasync.py 34.66% <0%> (+0.43%) ⬆️
airflow/hooks/dbapi_hook.py 91.73% <0%> (+0.82%) ⬆️
airflow/models/connection.py 77.4% <0%> (+0.96%) ⬆️
airflow/providers/apache/hive/hooks/hive.py 77.55% <0%> (+1.53%) ⬆️
airflow/plugins_manager.py 90.09% <0%> (+2.51%) ⬆️
airflow/serialization/serialized_objects.py 95.2% <0%> (+5.16%) ⬆️
airflow/providers/mysql/operators/mysql.py 100% <0%> (+45%) ⬆️
... and 2 more

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 27cc835...2440252. Read the comment docs.

@potiuk potiuk merged commit 945b988 into apache:master Feb 2, 2020
potiuk added a commit that referenced this pull request Feb 2, 2020
* Revert "[AIRFLOW-6662] Switch to --init docker flag for signal propagation (#7278)"

This reverts commit d1bf343.

* [AIRFLOW-6662] return back the dumb-init - installed by apt

We had stability problems with tests with --init flag so we are
going back to it

(cherry picked from commit 945b988)
kaxil pushed a commit that referenced this pull request Feb 3, 2020
* Revert "[AIRFLOW-6662] Switch to --init docker flag for signal propagation (#7278)"

This reverts commit d1bf343.

* [AIRFLOW-6662] return back the dumb-init - installed by apt

We had stability problems with tests with --init flag so we are
going back to it

(cherry picked from commit 945b988)
galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020
* Revert "[AIRFLOW-6662] Switch to --init docker flag for signal propagation (apache#7278)"

This reverts commit d1bf343.

* [AIRFLOW-6662] return back the dumb-init - installed by apt

We had stability problems with tests with --init flag so we are
going back to it
kaxil pushed a commit to astronomer/airflow that referenced this pull request Mar 30, 2020
* Revert "[AIRFLOW-6662] Switch to --init docker flag for signal propagation (apache#7278)"

This reverts commit d1bf343.

* [AIRFLOW-6662] return back the dumb-init - installed by apt

We had stability problems with tests with --init flag so we are
going back to it

(cherry picked from commit 945b988)
(cherry picked from commit 3ca84ec)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools area:Scheduler including HA (high availability) scheduler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants