-
Notifications
You must be signed in to change notification settings - Fork 637
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
updating changelogs and version to 1.4.0-0.23b0 #585
Conversation
9409f0a
to
7e9a191
Compare
7e9a191
to
130d607
Compare
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.
👍
-e "git+https://github.com/open-telemetry/opentelemetry-python-contrib.git@7e9a1915b6863681f01e264296282089a299d3d5#egg=opentelemetry-instrumentation&subdirectory=opentelemetry-instrumentation" | ||
-e "git+https://github.com/open-telemetry/opentelemetry-python.git#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk" | ||
-e "git+https://github.com/open-telemetry/opentelemetry-python-contrib.git#egg=opentelemetry-util-http&subdirectory=util/opentelemetry-util-http" | ||
-e "git+https://github.com/open-telemetry/opentelemetry-python-contrib.git@7e9a1915b6863681f01e264296282089a299d3d5#egg=opentelemetry-util-http&subdirectory=util/opentelemetry-util-http" |
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.
Why do these values have to change? Shouldn't it already get the correct packages from the CORE-PR that recently got merge (and which corresponds to this PR)?: open-telemetry/opentelemetry-python@bb41b81
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.
Well... This is a workaround to make tox -e docs
pass on the contrib repo 🤷
What happens is that after the latest release opentelemetry -sdk
now depends on opentelemetry-instrumentation
. The former is in the core repo and the latter was moved to the contrib one.
If we don't make these changes, then the docs
virtual environment will end up having opentelemetry-instrumentation
23dev0 installed (because right now the release branch of the contrib repo has not yet been merged into main
) and then will try to install opentelemetry-sdk
1.4.0 which depends on opentelemetry-instrumentation
23b0 (because right now the release branch of the core repo has been merged into main
).
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.
Thanks for explaining the issue! I took the time to look into and I think we can have a different solution that doesn't involved setting the SHA. I made a PR on your fork to solve this issue.
I actually don't even know why this works... the commit we're using here still has opentelemetry-instrumentation==0.23dev0
doesn't it? Would look to hear what you think but that has me confused 🙂
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.
What happens is that after the latest release
opentelemetry -sdk
now depends onopentelemetry-instrumentation
. The former is in the core repo and the latter was moved to the contrib one.
So core has a dependency on contrib now?
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.
Following up on my comment here, I made this PR on your fork to hopefully solve the docs issue in a different way.
I can't make the PR on opentelemetry-python-contrib
separately because it needs opentelemtry-instrument==0.23b0
which you are adding in this PR.
If we agree with my solution, we should update the PR description with my PR description for history purposes.
All right, marking this PR as draft while we look into your solution 👍 |
No description provided.