-
Notifications
You must be signed in to change notification settings - Fork 624
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove SDK dependency from opentelemetry-instrumentation-grpc #2474
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sdk is used directly in tests, should this be documented in the pyproject.toml?
This appear to work here:
|
Not really, the test dependencies are defined in the |
yeah but we don't have the sdk in test-requirements |
Yeah, that's an unfortunate consequence of our testing being done against a particular commit of the core repo, not a released version of the core repo. We have some test requirements in the |
@xrmx I think since That said, is there an issue with the tests using the SDK? That seems expected and the SDK is being installed in the test virtualenv |
I don't think there's an issue in using the sdk, I think there's an issue in not declaring the dependency explicitly. Said that probably running just pytest after installing the test requirements is not working already so no big deal. |
Ya I think Diego's comment is right, since it's in opentelemetry-python-contrib/tox.ini Line 444 in 5116305
|
…elemetry#2474) Co-authored-by: Diego Hurtado <ocelotl@users.noreply.github.com> Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
Description
Fixes #2473
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.