-
Notifications
You must be signed in to change notification settings - Fork 647
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
Use UV to install all needed packages locally #3124
base: main
Are you sure you want to change the base?
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.
Loved this. I would say other than speed-up local development, this paves the way to adopt uv in opentelemetry-python.
I think it will be necessary to add some instructions on how to use that. But just a note for reviewers:
With a simple command like uv sync
you can have all packages from the project installed locally. If you want to install only one package you can run: uv sync --package opentelemetry-instrumentation-sqlalchemy
for example.
Sure but aren't we adding another source of dependabot warnings? I'm fine on adding the metadata to the pyproject for people using uv but isn't this duplicating what we use tox for? |
Unfortunately/Fortunately, Dependabot doesn't see the
Not really. The idea here is to have a single virtual environment that can be used by the IDE, and to simplify local development (running some scripts locally to test some integrations, etc). The tox is used to run the tests with multiple different environments, it doesn't really create this virtual environment. |
Is it possible that |
This is not related with this PR, please create an issue. |
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.
I think you may need to regenerate the lockfile, it fails for me unless i regenerate with
AttributeError: module 'falcon' has no attribute '__version__'
hint: This usually indicates a problem with the package or the build environment.
help: `falcon` (v3.1.1) was included because `opentelemetry-python-contrib` (v0.0.0) depends on `opentelemetry-instrumentation-falcon[instruments]`
(v0.51b0.dev0) which depends on `falcon`
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 fails? Which command did you use?
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.
uv sync
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.
That updates the lock file.
I'll write some notes on how to use it.
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.
You'd need to do uv sync --frozen
, which doesn't update the lockfile.
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.
uv sync --frozen is failing for me too
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.
@Kludex could you please write a guide on how people can use this setup? Maybe a small section in CONTRIBUTING.md is enough.
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.
Looking forward to this. I have some awkward instructions on how to install things which I hope uv makes easier. Each time I use uv/uvx it seems quicker and simpler than before, so 👍
I'm ready. |
Looks like the uv.lock is confusing ruff? Also curious aws xray propagator test failures. |
No...
Calling the test ruff but running pre-commit doesn't help 👀 |
It seems it's looking to the |
@emdneto we talked about this at some point. Do you remember why? |
"instrumentation/*", | ||
"exporter/*", | ||
"opentelemetry-instrumentation", | ||
"propagator/*", |
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.
@Kludex According to documentation:
Within a workspace, dependencies on workspace members are facilitated via tool.uv.sources,
so since we have tests pointing to pinned versions of API (as example 1.16 for awsxray propagator) uv is trying to install it using the source at Git instead of PyPI and this version doesn't exist in ref main. I think this is happening because tox-uv is running based on what is defined at pyproject.toml
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.
Same will happen for all packages that are tested using pinned released versions of api/sdk in test requirements (example: gen-ai stuff)
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.
Instead of workspaces maybe we can have this for sources:
opentelemetry-propagator-aws-xray = { path = "./propagator/opentelemetry-propagator-aws-xray", editable = true }
opentelemetry-propagator-ot-trace = { path = "./propagator/opentelemetry-propagator-ot-trace", editable = true }
This is specifically to have a single virtual environment with all dependencies to make it easier to develop here.