Page MenuHomeSoftware Heritage

sentry: always override init settings with the environment variables
ClosedPublic

Authored by olasd on Apr 14 2022, 2:18 PM.

Details

Summary

Instead of only overriding the initial settings when they're left unset,
always override the defaults when the environment variables are set.

This makes the behavior more consistent with the way we usually handle
environment variables. It also allows setting the environment variable
SWH_SENTRY_DISABLE_LOGGING_EVENTS=false, to enable events based on the
logging framework in gunicorn backends (where the default has been
flipped to true).

Diff Detail

Event Timeline

Build has FAILED

Patch application report for D7580 (id=27455)

Rebasing onto 8f2cfa886c...

Current branch diff-target is up to date.
Changes applied before test
commit a3f3585a3c4bff8fe786474c40a30df6ae537f82
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 14 14:15:02 2022 +0200

    sentry: always override init settings with the environment variables
    
    Instead of only overriding the initial settings when they're left unset,
    always override the defaults when the environment variables are set.
    
    This makes the behavior more consistent with the way we usually handle
    environment variables. It also allows setting the environment variable
    `SWH_SENTRY_DISABLE_LOGGING_EVENTS=false`, to enable events based on the
    logging framework in gunicorn backends (where the default has been
    flipped to true).

Link to build: https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/398/
See console output for more information: https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/398/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 14 2022, 2:21 PM
Harbormaster failed remote builds in B28472: Diff 27455!
  • cli: Fork out tests that mess with the global logging setup

Build is green

Patch application report for D7580 (id=27460)

Rebasing onto 8f2cfa886c...

Current branch diff-target is up to date.
Changes applied before test
commit 851367d2ea8145d14ed3380686b35e0db81b2a7f
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 14 14:15:02 2022 +0200

    sentry: always override init settings with the environment variables
    
    Instead of only overriding the initial settings when they're left unset,
    always override the defaults when the environment variables are set.
    
    This makes the behavior more consistent with the way we usually handle
    environment variables. It also allows setting the environment variable
    `SWH_SENTRY_DISABLE_LOGGING_EVENTS=false`, to enable events based on the
    logging framework in gunicorn backends (where the default has been
    flipped to true).

commit 1b32b19eda75650de0180bad9cf67097268f58d8
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 14 15:19:52 2022 +0200

    cli: Fork out tests that mess with the global logging setup
    
    Notably, using logging.config.dictConfig disables all loggers before
    setting its own config. There's no simple way of undoing these changes
    to the logging config.
    
    Even if we had a way to reset a basic logging config, pytest reuses its
    log handler instances across tests, so it's not even that simple to be
    able to reset the logging config from scratch and still have pytest able
    to capture logs.

See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/399/ for more details.

olasd requested review of this revision.Apr 14 2022, 3:25 PM

lgtm, maybe add a comment in test_cli.py to justify the presence of forked over there

Switch approach for preventing changes to the global logging setup

Build is green

Patch application report for D7580 (id=27463)

Rebasing onto 8f2cfa886c...

Current branch diff-target is up to date.
Changes applied before test
commit 950b13e21e9b9aa48b1f191f51a6aea4787b2904
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 14 14:15:02 2022 +0200

    sentry: always override init settings with the environment variables
    
    Instead of only overriding the initial settings when they're left unset,
    always override the defaults when the environment variables are set.
    
    This makes the behavior more consistent with the way we usually handle
    environment variables. It also allows setting the environment variable
    `SWH_SENTRY_DISABLE_LOGGING_EVENTS=false`, to enable events based on the
    logging framework in gunicorn backends (where the default has been
    flipped to true).

commit b4e27788a60e78e4813faaa656cd35d1dcf60053
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 14 15:19:52 2022 +0200

    cli: Ensure tests don't mess with the global logging setup
    
    Notably, using logging.config.dictConfig disables all loggers before
    setting its own config. There's no simple way of undoing these changes
    to the logging config.
    
    Even if we had a way to reset a basic logging config, pytest reuses its
    log handler instances across tests, so it's not even that simple to be
    able to reset the logging config from scratch and still have pytest able
    to capture logs.
    
    Instead of any of that, just add a fixture to reset the root logger
    level, and to mock logging.config.dictConfig so that it doesn't actually
    get called.

See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/400/ for more details.

Make the cli tests clearer by running the asserts in the subcommand

Build is green

Patch application report for D7580 (id=27478)

Rebasing onto 8f2cfa886c...

Current branch diff-target is up to date.
Changes applied before test
commit 02cdfeb493a971bb5bebe0bd549c0da1e795e6e8
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 14 14:15:02 2022 +0200

    sentry: always override init settings with the environment variables
    
    Instead of only overriding the initial settings when they're left unset,
    always override the defaults when the environment variables are set.
    
    This makes the behavior more consistent with the way we usually handle
    environment variables. It also allows setting the environment variable
    `SWH_SENTRY_DISABLE_LOGGING_EVENTS=false`, to enable events based on the
    logging framework in gunicorn backends (where the default has been
    flipped to true).

commit 151a2f95c2670f50ad34e63227b10aa181a10922
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 14 15:19:52 2022 +0200

    cli: Ensure tests don't mess with the global logging setup
    
    Notably, using logging.config.dictConfig disables all loggers before
    setting its own config. There's no simple way of undoing these changes
    to the logging config.
    
    Even if we had a way to reset a basic logging config, pytest reuses its
    log handler instances across tests, so it's not even that simple to be
    able to reset the logging config from scratch and still have pytest able
    to capture logs.
    
    Instead of any of that, just add a fixture to reset the root logger
    level, and to mock logging.config.dictConfig so that it doesn't actually
    get called.

See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/401/ for more details.

This revision is now accepted and ready to land.Apr 15 2022, 12:42 PM

Add test to ensure that one can override the gunicorn (flipped) default with an envvar

Build is green

Patch application report for D7580 (id=27480)

Rebasing onto 8f2cfa886c...

Current branch diff-target is up to date.
Changes applied before test
commit c1b2e9371a7481d2eebdeb1dccfcdb64a29b62cb
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 14 14:15:02 2022 +0200

    sentry: always override init settings with the environment variables
    
    Instead of only overriding the initial settings when they're left unset,
    always override the defaults when the environment variables are set.
    
    This makes the behavior more consistent with the way we usually handle
    environment variables. It also allows setting the environment variable
    `SWH_SENTRY_DISABLE_LOGGING_EVENTS=false`, to enable events based on the
    logging framework in gunicorn backends (where the default has been
    flipped to true).

commit 151a2f95c2670f50ad34e63227b10aa181a10922
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Thu Apr 14 15:19:52 2022 +0200

    cli: Ensure tests don't mess with the global logging setup
    
    Notably, using logging.config.dictConfig disables all loggers before
    setting its own config. There's no simple way of undoing these changes
    to the logging config.
    
    Even if we had a way to reset a basic logging config, pytest reuses its
    log handler instances across tests, so it's not even that simple to be
    able to reset the logging config from scratch and still have pytest able
    to capture logs.
    
    Instead of any of that, just add a fixture to reset the root logger
    level, and to mock logging.config.dictConfig so that it doesn't actually
    get called.

See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/402/ for more details.