Skip to content

Conversation

@Maximilien-R
Copy link
Contributor

The goal is to import properly all context_providers to fix the import error when initializing a ddtrace.opentracer.Tracer object with a custom scope manager (as explained here).

@brettlangdon
Copy link
Member

Ha! You beat me to a PR, I basically had the exact same diff

@brettlangdon brettlangdon changed the title Fixing context provider imports for scope manager [opentracing] Fixing context provider imports for scope manager Dec 10, 2018
@brettlangdon brettlangdon added this to the 0.18.0 milestone Dec 10, 2018
@brettlangdon brettlangdon changed the base branch from master to 0.18-dev December 10, 2018 14:52
@brettlangdon brettlangdon modified the milestones: 0.18.0, 0.18.1 Dec 10, 2018
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

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

Nice 😄. We can probably also add a regression test for this since we should cover when custom scope managers are used. But I'm fine leaving that for a separate PR too.

@Maximilien-R
Copy link
Contributor Author

@Kyle-Verhoog The fact is that the get_context_provider_for_scope_manager function already have unit tests (both Gevent & Asyncio). The tests don't fail certainly because before their execution there is an "explicit" import of their context_provider which put them in the python import cache.

@brettlangdon brettlangdon changed the base branch from 0.18-dev to 0.19-dev December 12, 2018 20:56
@brettlangdon brettlangdon modified the milestones: 0.18.1, 0.19.0 Dec 20, 2018
@brettlangdon brettlangdon merged commit 8e679bd into DataDog:0.19-dev Dec 20, 2018
@brettlangdon brettlangdon mentioned this pull request Dec 28, 2018
@brettlangdon
Copy link
Member

@Maximilien-R thank you for this contribution, we just released version 0.19.0 which includes this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants