-
Notifications
You must be signed in to change notification settings - Fork 414
fix: Resolve uv.lock conflicting dependencies with semgrep #1234
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
fix: Resolve uv.lock conflicting dependencies with semgrep #1234
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.
Pull Request Overview
This PR resolves dependency conflicts between semgrep and OpenTelemetry observability packages by restructuring dependency management and adopting uv/uvx for isolated execution.
- Restructured
pyproject.tomlto useuv's[dependency-groups]format and removed conflicting observability exporter extras - Updated Makefile to use
uvx semgrepfor isolated execution anduv run pytestfor testing - Enhanced observability documentation with detailed backend descriptions and installation guidance
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pyproject.toml | Restructured dependency management, moved dev dependencies to [dependency-groups], removed conflicting observability exporters, commented out semgrep dependency |
| docs/docs/manage/observability/observability.md | Expanded backend descriptions, added installation instructions per backend, clarified mutual exclusivity of exporters |
| Makefile | Updated to use uvx semgrep for isolated execution, uv run pytest for testing, and conditional uv installation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Looking at what's changing the pytest configuration -- it has to do with removing the "dev" packaging from the optional packages. There's a good reason for that -- the list of dev dependencies doesn't ship with the production package. The down side is the pytest environment needs a different approach to installing dev/test dependencies. |
8340309 to
26e7971
Compare
|
@crivetimihai Are you able to trigger a re-run of the one failed job? It looks like the failure was from resource contention to me. |
This commit addresses dependency resolution conflicts between semgrep and OpenTelemetry packages by: 1. **Makefile improvements**: - Updated venv target to check if uv is already installed system-wide - Modified test target to use `uv run pytest` instead of installing pytest separately - Fixed semgrep target to use `uvx semgrep` to avoid dependency conflicts - Removed syntax error in lint-smart target (extra `fi` statement) 2. **pyproject.toml restructuring**: - Moved development dependencies from [project.optional-dependencies] to [dependency-groups] - Removed semgrep from dev dependencies (now run via uvx to avoid conflicts) - Simplified observability extras to only include core OpenTelemetry packages - Removed observability-jaeger, observability-zipkin, and observability-all extras - Removed dev-all extras group 3. **Documentation updates** (docs/docs/manage/observability/observability.md): - Expanded backend options with detailed descriptions of Arize Phoenix, Jaeger, and Grafana Tempo - Added clear guidance on exporter backend installation per platform - Clarified that only one exporter backend should be installed at a time - Improved formatting and structure for better readability 4. **uv.lock regeneration**: - Updated lock file to reflect new dependency structure - Resolved conflicts between semgrep and opentelemetry-sdk packages The changes transition towards using uv/uvx for dependency management while maintaining backward compatibility with existing workflows. Signed-off-by: Jon Pspri <[email protected]> Signed-off-by: Jonathan Springer <[email protected]> Signed-off-by: Jonathan Springer <[email protected]>
- Update .github/workflows/pytest.yml to use uv instead of pip - Fix Makefile venv target shell syntax for uv installation - Update type hints for keep_alive parameter (int -> float) - Improve test_translate_echo.py with proper type annotations - Add hypothesis to dev dependencies for property-based testing - Use unused_tcp_port fixture to prevent port conflicts Resolves conflicting dependencies in uv.lock and improves type safety. Signed-off-by: Jonathan Springer <[email protected]>
|
Yep... All reviews passed. |
7a33f11 to
6ddabaa
Compare
|
Rebase and push with a tweak to get that '|| true' that Copilot so loves back into the codebase. |
Signed-off-by: Jonathan Springer <[email protected]>
6ddabaa to
2491595
Compare
|
Sorry for all the pushes... my branch got polluted with some other things I was working on. |
|
@jonpspri is this rebased and ready to merge, or does it have other files that should be a separate PR? |
|
@crivetimihai It should be GTG. Everything in it now is related to resolving the semgrep/observability version conflicts. |
* fix: Resolve uv.lock conflicting dependencies with semgrep This commit addresses dependency resolution conflicts between semgrep and OpenTelemetry packages by: 1. **Makefile improvements**: - Updated venv target to check if uv is already installed system-wide - Modified test target to use `uv run pytest` instead of installing pytest separately - Fixed semgrep target to use `uvx semgrep` to avoid dependency conflicts - Removed syntax error in lint-smart target (extra `fi` statement) 2. **pyproject.toml restructuring**: - Moved development dependencies from [project.optional-dependencies] to [dependency-groups] - Removed semgrep from dev dependencies (now run via uvx to avoid conflicts) - Simplified observability extras to only include core OpenTelemetry packages - Removed observability-jaeger, observability-zipkin, and observability-all extras - Removed dev-all extras group 3. **Documentation updates** (docs/docs/manage/observability/observability.md): - Expanded backend options with detailed descriptions of Arize Phoenix, Jaeger, and Grafana Tempo - Added clear guidance on exporter backend installation per platform - Clarified that only one exporter backend should be installed at a time - Improved formatting and structure for better readability 4. **uv.lock regeneration**: - Updated lock file to reflect new dependency structure - Resolved conflicts between semgrep and opentelemetry-sdk packages The changes transition towards using uv/uvx for dependency management while maintaining backward compatibility with existing workflows. Signed-off-by: Jon Pspri <[email protected]> Signed-off-by: Jonathan Springer <[email protected]> Signed-off-by: Jonathan Springer <[email protected]> * fix: Migrate pytest workflow to uv and resolve type hints - Update .github/workflows/pytest.yml to use uv instead of pip - Fix Makefile venv target shell syntax for uv installation - Update type hints for keep_alive parameter (int -> float) - Improve test_translate_echo.py with proper type annotations - Add hypothesis to dev dependencies for property-based testing - Use unused_tcp_port fixture to prevent port conflicts Resolves conflicting dependencies in uv.lock and improves type safety. Signed-off-by: Jonathan Springer <[email protected]> * fix: Use || true instead of ||: Signed-off-by: Jonathan Springer <[email protected]> --------- Signed-off-by: Jon Pspri <[email protected]> Signed-off-by: Jonathan Springer <[email protected]> Signed-off-by: Jonathan Springer <[email protected]> Signed-off-by: p4yl04d3r <[email protected]>
Summary
This PR resolves issue #1230 by addressing dependency resolution conflicts between
semgrepand OpenTelemetry observability exporter packages. The root cause was thatsemgrepand variousopentelemetry-exporter-*packages had incompatible dependency versions that preventeduvfrom successfully resolving the dependency tree.Root Cause
The
semgreppackage and OpenTelemetry exporter packages (opentelemetry-exporter-jaeger,opentelemetry-exporter-zipkin,opentelemetry-exporter-otlp-proto-grpc) had conflicting dependencies that created an unresolvable dependency graph inuv.lock.Solution
This PR restructures dependency management to eliminate the conflicts:
1. Makefile improvements
venvtarget to check ifuvis already installed system-wide before installing ittesttarget to useuv run pytestinstead of installing pytest separatelysemgreptarget to useuvx semgrepto run semgrep in an isolated environment, avoiding dependency conflictslint-smarttarget (extrafistatement)2. pyproject.toml restructuring
[project.optional-dependencies]to[dependency-groups](uv's preferred format)semgrepfrom dev dependencies - now run viauvxwhich isolates its dependenciesobservabilityextras to only include core OpenTelemetry packages (opentelemetry-api,opentelemetry-sdk)observability-jaeger,observability-zipkin, andobservability-alldev-allextras group3. Documentation updates (observability.md)
4. uv.lock regeneration
Test Plan
uv lockto verify dependency resolution succeedsmake venvto verify virtual environment creation worksmake testto verify tests run withuv run pytestmake semgrepto verify semgrep runs viauvxin isolationuv.lockImpact
This change transitions towards using
uv/uvxfor dependency management while maintaining backward compatibility with existing workflows. By runningsemgrepviauvxand simplifying observability extras, users can now install the gateway and its optional features without encountering dependency conflicts.Closes #1230