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

Fix error when running tasks with Sentry integration enabled. #13929

Merged
merged 4 commits into from
Mar 19, 2021

Conversation

junnplus
Copy link
Contributor

closes #13797


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
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.

@github-actions
Copy link

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions
Copy link

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@ashb ashb added this to the Airflow 2.0.1 milestone Feb 2, 2021
airflow/sentry.py Outdated Show resolved Hide resolved
@junnplus junnplus force-pushed the patch-sentry branch 2 times, most recently from 2e1cbed to 910a443 Compare February 4, 2021 17:22
@junnplus junnplus changed the title Remove redundant session arg Parse session from positional/kwargs argument Feb 4, 2021
airflow/sentry.py Outdated Show resolved Hide resolved
airflow/sentry.py Outdated Show resolved Hide resolved
airflow/utils/session.py Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Feb 4, 2021

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@junnplus junnplus force-pushed the patch-sentry branch 2 times, most recently from 93cb7bc to d8e1216 Compare February 5, 2021 01:48
@junnplus junnplus requested a review from ashb February 5, 2021 01:50
@github-actions
Copy link

github-actions bot commented Feb 5, 2021

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions
Copy link

github-actions bot commented Feb 5, 2021

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

airflow/sentry.py Show resolved Hide resolved
@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Feb 5, 2021
@github-actions
Copy link

github-actions bot commented Feb 5, 2021

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@junnplus
Copy link
Contributor Author

junnplus commented Feb 5, 2021

@ashb Some new checks were not successful.
Seems mock.create_autospec has a problem when python version < 3.7.3, I can skip these test?

@ashb
Copy link
Member

ashb commented Feb 5, 2021

The new tests you added are failing though.

@junnplus junnplus force-pushed the patch-sentry branch 2 times, most recently from 9dcaf3e to ab42904 Compare February 5, 2021 15:38
@github-actions
Copy link

github-actions bot commented Feb 5, 2021

The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.

@junnplus junnplus force-pushed the patch-sentry branch 2 times, most recently from 3fd7ea6 to d0ecc51 Compare February 7, 2021 03:45
@junnplus
Copy link
Contributor Author

junnplus commented Feb 7, 2021

@ashb I mark the 3.6 test as skipped. Please check again, thanks.

@kaxil kaxil removed this from the Airflow 2.0.1 milestone Feb 9, 2021
@github-actions
Copy link

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@junnplus junnplus force-pushed the patch-sentry branch 3 times, most recently from 7841a57 to 00bc83d Compare February 28, 2021 03:15
@junnplus junnplus force-pushed the patch-sentry branch 3 times, most recently from 31a1cf2 to 4e12b50 Compare March 5, 2021 05:31
@junnplus junnplus requested a review from ashb March 5, 2021 07:25
@junnplus
Copy link
Contributor Author

junnplus commented Mar 9, 2021

@ashb @kaxil We can merge this PR?

@ashb ashb changed the title Parse session from positional/kwargs argument Fix error when running tasks with Sentry integration enabled. Mar 9, 2021
@ashb ashb merged commit 0e8698d into apache:master Mar 19, 2021
ashb pushed a commit that referenced this pull request Mar 19, 2021
Co-authored-by: Ash Berlin-Taylor <[email protected]>
(cherry picked from commit 0e8698d)
ashb pushed a commit that referenced this pull request Apr 15, 2021
Co-authored-by: Ash Berlin-Taylor <[email protected]>
(cherry picked from commit 0e8698d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sentry celery dag task run error
4 participants