cassandra: use _instruments_any instead of _instruments for driver deps#4182
Conversation
fdcc855 to
b27f1e3
Compare
935feab to
f96edf4
Compare
xrmx
left a comment
There was a problem hiding this comment.
Thanks for the PR, I think you are missing one piece of the puzzle that is to test both the implementation and not just one.
In short you have to split the test-requirements.txt in one for cassandra-driver and the other for scylla-driver and update tox.ini accordingly.
The split plus a ton of unrelated changes are available in this PR https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4180/changes
f96edf4 to
51d855b
Compare
andreamatt
left a comment
There was a problem hiding this comment.
Done — split test requirements into separate files for cassandra-driver and scylla-driver, with separate tox environments for each. Both pass:
py313-test-instrumentation-cassandra-driver: 8/8 passedpy312-test-instrumentation-cassandra-scylla: 8/8 passed
51d855b to
7ba9c50
Compare
The cassandra instrumentation listed both cassandra-driver and scylla-driver in _instruments, which requires ALL listed packages to be installed. Since users install one OR the other (they are alternative drivers for the same API), this should use _instruments_any so that having either driver installed is sufficient. This follows the same pattern already used by psycopg2 and kafka-python instrumentations, introduced in open-telemetry#3610. Co-authored-by: Cursor <cursoragent@cursor.com>
7ba9c50 to
de3f0f8
Compare
Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com>
…/opentelemetry-python-contrib into fix/cassandra-instruments-any
|
Opened a PR for updating the pending checks with the renamed ones, need that to be merged before we can merge this. |
|
Uhm the terraform definition looks like this: But the for some reasons the two contexts not referring to a specific tox environment are not found for this instrumentation but they are found for the other instrumentations 😓 |
|
The names without the python version are the lint ones and the one for the cassandra instrumentation is called |
…ps (open-telemetry#4182) * cassandra: use _instruments_any instead of _instruments for driver deps The cassandra instrumentation listed both cassandra-driver and scylla-driver in _instruments, which requires ALL listed packages to be installed. Since users install one OR the other (they are alternative drivers for the same API), this should use _instruments_any so that having either driver installed is sufficient. This follows the same pattern already used by psycopg2 and kafka-python instrumentations, introduced in open-telemetry#3610. Co-authored-by: Cursor <cursoragent@cursor.com> * fix ci Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> * fix uv.lock Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> --------- Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: emdneto <9735060+emdneto@users.noreply.github.com>
…ps (open-telemetry#4182) * cassandra: use _instruments_any instead of _instruments for driver deps The cassandra instrumentation listed both cassandra-driver and scylla-driver in _instruments, which requires ALL listed packages to be installed. Since users install one OR the other (they are alternative drivers for the same API), this should use _instruments_any so that having either driver installed is sufficient. This follows the same pattern already used by psycopg2 and kafka-python instrumentations, introduced in open-telemetry#3610. Co-authored-by: Cursor <cursoragent@cursor.com> * fix ci Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> * fix uv.lock Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> --------- Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: emdneto <9735060+emdneto@users.noreply.github.com>
…ps (open-telemetry#4182) * cassandra: use _instruments_any instead of _instruments for driver deps The cassandra instrumentation listed both cassandra-driver and scylla-driver in _instruments, which requires ALL listed packages to be installed. Since users install one OR the other (they are alternative drivers for the same API), this should use _instruments_any so that having either driver installed is sufficient. This follows the same pattern already used by psycopg2 and kafka-python instrumentations, introduced in open-telemetry#3610. Co-authored-by: Cursor <cursoragent@cursor.com> * fix ci Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> * fix uv.lock Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> --------- Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: emdneto <9735060+emdneto@users.noreply.github.com>
Description
The cassandra instrumentation listed both
cassandra-driverandscylla-driverin_instruments, which requires all listed packages to be installed. Since users install one or the other (they are alternative drivers for the samecassandra.clusterAPI), this should use_instruments_anyso that having either driver installed is sufficient.This follows the same pattern already used by the psycopg2 and kafka-python instrumentations, introduced in #3610. Cassandra was the only remaining instrumentation with multiple alternative packages listed under
_instrumentsinstead of_instruments_any.Also splits test requirements into separate files for each driver and adds separate tox/CI environments to test both independently.
Fixes #2819
Type of change
How Has This Been Tested?
DependencyConflict: requested: "scylla-driver ~= 3.25" but found: "None"when onlycassandra-driveris installed (3/4 tests fail)instrumentation_dependencies()covering all four cases: only cassandra-driver, only scylla-driver, both installed, neither installedpy313-test-instrumentation-cassandra-driver: 8/8 passedpy312-test-instrumentation-cassandra-scylla: 8/8 passedtox -e generateandtox -e generate-workflowsto regenerate filesDoes This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.