Skip to content
This repository has been archived by the owner on Oct 8, 2024. It is now read-only.

solution and test for tracer multi-import issue #161

Merged
merged 5 commits into from
Aug 16, 2024

Conversation

PietroPasotti
Copy link
Contributor

Issue

Fixes #60

Solution

Added a warning if we're detecting a double import situation.

Context

https://python-notes.curiousefficiency.org/en/latest/python_concepts/import_traps.html#the-double-import-trap

Testing Instructions

Added a test verifying the warning behaviour.

Copy link
Contributor

@ca-scribner ca-scribner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaict this makes sense, but I am not so well versed so a second opinion might be warranted.

lib/charms/tempo_k8s/v1/charm_tracing.py Outdated Show resolved Hide resolved
tests/scenario/test_a_charm_tracer_multi_import_warning.py Outdated Show resolved Hide resolved
tests/scenario/test_a_charm_tracer_multi_import_warning.py Outdated Show resolved Hide resolved
@PietroPasotti PietroPasotti merged commit abd2ff6 into main Aug 16, 2024
13 checks passed
@PietroPasotti PietroPasotti deleted the fix-tracer-missing-from-context branch August 16, 2024 12:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracer contextvar is sometimes empty
2 participants