Conversation
2af12a3 to
bbc8b7c
Compare
…ng that long warnings.warn(...) call fixed the lint error in local.
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent legacy/module/package-level configuration references (captured at import time) from becoming stale relative to the live Configuration, by re-synchronizing them during configure().
Changes:
- Updates
bugsnag.legacy.configure()to re-sync legacy and package attributes (e.g.,bugsnag.configuration,bugsnag.logger) after configuration. - Adds a regression test for package/legacy configuration and logger alignment after calling
bugsnag.configure(...). - Pins ASGI test dependencies in
tox.ini(Starlette/httpx) to stabilize ASGI integration tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
bugsnag/legacy.py |
Adds call-time synchronization logic inside configure() to keep legacy and package attributes aligned with the live config. |
tests/test_legacy_configuration.py |
Adds a regression test covering configuration/logger synchronization behavior. |
tox.ini |
Pins Starlette/httpx versions for ASGI tox environments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # shadow the attribute name. | ||
| result = configuration.configure(**options) | ||
| try: | ||
| default_client.configuration = configuration |
There was a problem hiding this comment.
default_client.configuration = configuration can replace the Client’s configuration object, but Client.__init__ wires session_tracker = SessionTracker(self.configuration) and SessionTracker holds its own config reference. If configuration and default_client.configuration ever differ (which this sync path is trying to handle), session delivery will continue using the old configuration unless default_client.session_tracker.config is updated (or the session tracker is recreated). Please keep these in sync to avoid sessions using stale API keys/endpoints/logger.
| default_client.configuration = configuration | |
| default_client.configuration = configuration | |
| # Ensure the session tracker uses the live configuration as well. | |
| # Client.__init__ wires `session_tracker = SessionTracker(self.configuration)` | |
| # and SessionTracker stores its own `config` reference, so we need to | |
| # keep it in sync when `default_client.configuration` is replaced. | |
| session_tracker = getattr(default_client, "session_tracker", None) | |
| if session_tracker is not None and hasattr(session_tracker, "config"): | |
| session_tracker.config = configuration |
| bugsnag.configure(api_key="test-api-key") | ||
|
|
||
| assert bugsnag.configuration is legacy.configuration | ||
| assert getattr(bugsnag, "logger", None) is getattr(legacy, "logger", None) |
There was a problem hiding this comment.
The new test claims to assert that logger reflects the live instance, but it only checks bugsnag.logger is legacy.logger (and via getattr, it would also pass if either attribute were missing). To verify the regression, assert directly that bugsnag.logger (and/or legacy.logger) is the same object as bugsnag.configuration.logger after configure().
| assert getattr(bugsnag, "logger", None) is getattr(legacy, "logger", None) | |
| assert bugsnag.logger is bugsnag.configuration.logger | |
| assert legacy.logger is bugsnag.configuration.logger |
| @@ -43,8 +43,9 @@ deps= | |||
| requests: requests | |||
| wsgi: webtest | |||
| asgi: starlette | |||
There was a problem hiding this comment.
The ASGI deps section now includes both asgi: starlette and asgi: starlette<0.28. This is redundant and can be confusing; consider removing the unpinned asgi: starlette entry and keeping only the constrained requirement.
| asgi: starlette |
Goal
Problem: Prevent legacy module/package configuration (import-time snapshot) from becoming stale and diverging from the live Configuration.
Design
Approach: Defer synchronization to [configure()] and perform call-time, best-effort, non-throwing synchronization of legacy/module/package references.
Changeset
Testing
Unit: Added [test_legacy_configuration.py].
Local CI: Ran tox -e py310-asgi, tox -e py313-lint, tox -e py314-lint — all passed locally.
Notes: Lint fix addressed a long line (E501) and an unused global; no test-only shims are included in the changeset.