Visitar URL original
Explain sentry default environment variable for subprocess hook by potiuk · Pull Request #18346 · apache/airflow · GitHub
Skip to content

Explain sentry default environment variable for subprocess hook - #18346

Merged
potiuk merged 2 commits into
apache:mainfrom
potiuk:disable-default-integrations-in-sentry-tests
Sep 18, 2021
Merged

potiuk merged 2 commits into
apache:mainfrom
potiuk:disable-default-integrations-in-sentry-tests

Conversation

@potiuk

@potiuk potiuk commented Sep 18, 2021

Copy link
Copy Markdown
Member

Sentry by default monkey-patches standard library POpen function
to capture and pass current process' environment to subprocess
with SUBPROCESS_ prefix. This break SubprocessHook tests
when sentry tests are run before SubprocessHook tests, and also
it modifies SubprocessHook behaviour (and promise) in production
environment as well.

This PR:

  • adds documentation to both sentry documentation and the
    SubprocessHook documentation explaining the interaction between
    the two
  • Adds documentation explaining how to disable this default
    Sentry behaviour
  • disables default integrations in the Sentry tests to avoid
    side-effects

Fixes: #18268


^ 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.

Sentry by default monkey-patches standard library POpen function
to capture and pass current process' environment to subprocess
with `SUBPROCESS_` prefix. This break SubprocessHook tests
when sentry tests are run before SubprocessHook tests, and also
it modifies SubprocessHook behaviour (and promise) in production
environment as well.

This PR:

* adds documentation to both sentry documentation and the
  SubprocessHook documentation explaining the interaction between
  the two
* Adds documentation explaining how to disable this default
  Sentry behaviour
* disables default integrations in the Sentry tests to avoid
  side-effects

Fixes: apache#18268
@potiuk

potiuk commented Sep 18, 2021

Copy link
Copy Markdown
Member Author

cc: @ecerulm

Co-authored-by: Xiaodong DENG <xd.deng.r@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

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 main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions Bot added the full tests needed We need to run full set of tests for this PR to merge label Sep 18, 2021
@potiuk
potiuk merged commit 50e2e6e into apache:main Sep 18, 2021
@potiuk
potiuk deleted the disable-default-integrations-in-sentry-tests branch September 18, 2021 17:16
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 kind:documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_sentry.py breaks many other tests

3 participants