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-3600] Remove dagbag from trigger #4407

Merged
merged 6 commits into from
Dec 31, 2018

Conversation

ffinfo
Copy link
Contributor

@ffinfo ffinfo commented Dec 30, 2018

Make sure you have checked all steps below.

Jira

Description

Removed DagBag from trigger call

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.

Code Quality

  • Passes flake8

@codecov-io
Copy link

codecov-io commented Dec 30, 2018

Codecov Report

Merging #4407 into master will increase coverage by 0.45%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #4407      +/-   ##
=========================================
+ Coverage   78.14%   78.6%   +0.45%     
=========================================
  Files         204     204              
  Lines       16433   16445      +12     
=========================================
+ Hits        12842   12926      +84     
+ Misses       3591    3519      -72
Impacted Files Coverage Δ
airflow/www/views.py 70.24% <100%> (+0.77%) ⬆️
airflow/www_rbac/views.py 73.71% <100%> (+0.93%) ⬆️
airflow/models/__init__.py 92.55% <100%> (-0.02%) ⬇️
airflow/hooks/samba_hook.py 38.88% <0%> (+38.88%) ⬆️
airflow/operators/hive_to_samba_operator.py 100% <0%> (+100%) ⬆️
airflow/operators/jdbc_operator.py 100% <0%> (+100%) ⬆️

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 53e7074...0957961. Read the comment docs.

@Fokko
Copy link
Contributor

Fokko commented Dec 31, 2018

@ffinfo can you explain the reasoning behind this PR?

@Fokko Fokko merged commit 548f38a into apache:master Dec 31, 2018
@ffinfo
Copy link
Contributor Author

ffinfo commented Dec 31, 2018

@Fokko I did forgot the link in jira for this. This is part of AIRFLOW-3562

@Fokko
Copy link
Contributor

Fokko commented Dec 31, 2018

Ok, thanks!

aliceabe pushed a commit to aliceabe/incubator-airflow that referenced this pull request Jan 3, 2019
* Remove dagbag from trigger call

* Adding fix to rbac

* empty commit

* Added create_dagrun to DagModel

* Adding testing to /trigger calls

* Make session a class var
yohei1126 pushed a commit to yohei1126/airflow that referenced this pull request Jan 6, 2019
* Remove dagbag from trigger call

* Adding fix to rbac

* empty commit

* Added create_dagrun to DagModel

* Adding testing to /trigger calls

* Make session a class var
yohei1126 pushed a commit to yohei1126/airflow that referenced this pull request Jan 6, 2019
* Remove dagbag from trigger call

* Adding fix to rbac

* empty commit

* Added create_dagrun to DagModel

* Adding testing to /trigger calls

* Make session a class var
yohei1126 pushed a commit to yohei1126/airflow that referenced this pull request Jan 6, 2019
* Remove dagbag from trigger call

* Adding fix to rbac

* empty commit

* Added create_dagrun to DagModel

* Adding testing to /trigger calls

* Make session a class var
yohei1126 pushed a commit to yohei1126/airflow that referenced this pull request Jan 6, 2019
* Remove dagbag from trigger call

* Adding fix to rbac

* empty commit

* Added create_dagrun to DagModel

* Adding testing to /trigger calls

* Make session a class var
@feng-tao
Copy link
Member

feng-tao commented Jan 7, 2019

@ffinfo , @Fokko , it seems that trigger_dag test is quite flaky(test_trigger_dag_button), I have seen it fails in couples of prs(e.g: https://travis-ci.org/apache/airflow/jobs/476146347) especially on mysql ORM. Do you guys know what is happening?

@feng-tao
Copy link
Member

feng-tao commented Jan 7, 2019

@ffinfo , please let me know if you will investigate the issue. If not, I prefer to revert this change until the flaky test is fixed. What do you think @Fokko ?

@Fokko
Copy link
Contributor

Fokko commented Jan 7, 2019

======================================================================
42) FAIL: test_trigger_dag_button (tests.www_rbac.test_views.TestTriggerDag)
----------------------------------------------------------------------
   Traceback (most recent call last):
    tests/www_rbac/test_views.py line 1476 in test_trigger_dag_button
      self.assertIsNotNone(run)
   AssertionError: unexpectedly None

Hmm, are you sure that this PR is the source?

@feng-tao
Copy link
Member

feng-tao commented Jan 7, 2019

@Fokko , looking at recent commit, this is the one that modifies this part of the code. And the CI is not always fails with this test(sometimes works, sometimes not). Hence I suspect this pr is the case.

And we are not sure when this pr checked in CI is broken or not, right?

@feng-tao
Copy link
Member

feng-tao commented Jan 8, 2019

another failure in https://travis-ci.org/apache/airflow/jobs/476638427 for #4436. It seems the test fails very consistently with mysql ORM.

ashb pushed a commit to ashb/airflow that referenced this pull request Mar 20, 2019
* Remove dagbag from trigger call

* Adding fix to rbac

* empty commit

* Added create_dagrun to DagModel

* Adding testing to /trigger calls

* Make session a class var
wmorris75 pushed a commit to modmed/incubator-airflow that referenced this pull request Jul 29, 2019
* Remove dagbag from trigger call

* Adding fix to rbac

* empty commit

* Added create_dagrun to DagModel

* Adding testing to /trigger calls

* Make session a class var
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants