Skip to content

fix(python): install workspace prerequisites before instrumentors - #3744

Open
mikeldking wants to merge 2 commits into
mainfrom
fix/python-ci-workspace-prerequisites
Open

mikeldking wants to merge 2 commits into
mainfrom
fix/python-ci-workspace-prerequisites

Conversation

@mikeldking

@mikeldking mikeldking commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Closes #3743.

Install checked-out semantic conventions and core instrumentation as bookends around leaf-specific setup in tox commands_pre:

  1. before leaf installation, so new internal minimums resolve from the checkout while UV_EXCLUDE_NEWER hides recent PyPI releases;
  2. after test/latest dependency changes, so CI tests checked-out source with its declared dependency versions.

Verification:

  • py310-ci-agno-latest: 147 tests passed
  • py314-ci-mistralai-latest: 25 tests passed
  • both previously failing environments resolve local openinference-instrumentation==0.1.63.

@github-actions

Copy link
Copy Markdown
Contributor

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Comment thread python/tox.ini
Comment on lines +299 to +300
uv pip install --reinstall {toxinidir}/openinference-semantic-conventions
uv pip install --reinstall {toxinidir}/openinference-instrumentation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The trailing unconditional reinstall clobbers the [test] extras for the instrumentation environment.

In the new ordering, the execution sequence for the instrumentation tox environment is:

  1. Line ~107: uv pip install --reinstall {toxinidir}/openinference-instrumentation (unconditional — no extras)
  2. Line ~256: instrumentation: uv pip install --reinstall {toxinidir}/openinference-instrumentation[test] (factor-gated — installs [test] extras ✓)
  3. Line 300: uv pip install --reinstall {toxinidir}/openinference-instrumentation (unconditional — overwrites without [test] extras ✗)

In the original code, the two unconditional installs were in the middle of commands_pre, so the factor-gated [test] install at line 256 came last and won. Now they're bookends, and the trailing unconditional (line 300) runs after the [test] install, stripping the extras.

Fix: add a factor-gated reinstall after the unconditional trailing line so the instrumentation environment ends up with [test] extras:

Suggested change
uv pip install --reinstall {toxinidir}/openinference-semantic-conventions
uv pip install --reinstall {toxinidir}/openinference-instrumentation
uv pip install --reinstall {toxinidir}/openinference-semantic-conventions
uv pip install --reinstall {toxinidir}/openinference-instrumentation
instrumentation: uv pip install --reinstall {toxinidir}/openinference-instrumentation[test]

This branch has not been deployed

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Python CI must install local workspace prerequisites before instrumentors

1 participant