Skip to content

Commit 15f4bda

Browse files
authored
fix: Allow to correctly compare parsed versions (getsentry#7184)
Originally raised by a bot [here](getsentry#5005 (comment)): the `parse_version` function parses version strings as is (e.g. 3.1 becomes `(3, 1)`). We use these parsed version tuples in integrations to compare the installed version against the minimum (defined in `integrations/__init__.py`). The minimum versions are often three-part, e.g. `(3, 1, 0)`. This means that we can mistakenly consider a valid version to be below the minimum, because in pure tuple terms, `(3, 1) < (3, 1, 0)` is true. This can also happen in reverse (package version has three parts, while our min version boundary has two). In this PR, we make the internal version comparison work as expected regardless of mismatches in the length of the version strings/tuples.
1 parent 954f6a7 commit 15f4bda

3 files changed

Lines changed: 67 additions & 7 deletions

File tree

‎sentry_sdk/integrations/__init__.py‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from abc import ABC, abstractmethod
2+
from itertools import zip_longest
23
from threading import Lock
34
from typing import TYPE_CHECKING
45

@@ -299,10 +300,17 @@ def _check_minimum_version(
299300
if min_version is None:
300301
return
301302

302-
if version < min_version:
303-
raise DidNotEnable(
304-
f"Integration only supports {package} {'.'.join(map(str, min_version))} or newer."
305-
)
303+
# We can't use normal tuple comparison here because the version tuples might
304+
# not have the same length, in which case they wouldn't compare as expected.
305+
for v, min in zip_longest(version, min_version, fillvalue=0):
306+
if v == min:
307+
continue
308+
elif v < min:
309+
raise DidNotEnable(
310+
f"Integration only supports {package} {'.'.join(map(str, min_version))} or newer."
311+
)
312+
elif v > min:
313+
return
306314

307315

308316
class DidNotEnable(Exception): # noqa: N818

‎sentry_sdk/integrations/celery/__init__.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
capture_internal_exceptions,
2525
event_from_exception,
2626
has_data_collection_enabled,
27+
parse_version,
2728
reraise,
2829
)
2930

@@ -36,7 +37,7 @@
3637

3738

3839
try:
39-
from celery import VERSION as CELERY_VERSION # type: ignore
40+
from celery import __version__ as CELERY_VERSION # type: ignore
4041
from celery.app.task import Task # type: ignore
4142
from celery.app.trace import task_has_custom
4243
from celery.exceptions import ( # type: ignore
@@ -73,7 +74,7 @@ def __init__(
7374

7475
@staticmethod
7576
def setup_once() -> None:
76-
_check_minimum_version(CeleryIntegration, CELERY_VERSION)
77+
_check_minimum_version(CeleryIntegration, parse_version(CELERY_VERSION))
7778

7879
_patch_build_tracer()
7980
_patch_task_apply_async()

‎tests/test_utils.py‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import sentry_sdk
1010
from sentry_sdk._queue import Queue
11-
from sentry_sdk.integrations import Integration
11+
from sentry_sdk.integrations import DidNotEnable, Integration, _check_minimum_version
1212
from sentry_sdk.utils import (
1313
Components,
1414
Dsn,
@@ -556,6 +556,57 @@ def test_parse_version(version, expected_result):
556556
assert parse_version(version) == expected_result
557557

558558

559+
@pytest.mark.parametrize(
560+
"version,min_version,expected_pass",
561+
[
562+
("1.0.0", (1, 0, 0), True),
563+
("1.0.1", (2, 0, 0), False),
564+
("1", (1, 0, 2), False),
565+
("1.0", (1, 0, 2), False),
566+
("1.0.1", (1, 0, 2), False),
567+
("1.0.1", (1, 0, 1), True),
568+
("1.0", (2,), False),
569+
(
570+
"1.0.1",
571+
(
572+
2,
573+
0,
574+
),
575+
False,
576+
),
577+
("1.0.1", (2, 0, 0), False),
578+
("2.0", (1,), True),
579+
(
580+
"2.0.1",
581+
(
582+
1,
583+
1,
584+
),
585+
True,
586+
),
587+
("2.0.1", (1, 1, 2), True),
588+
("1", (1, 0), True),
589+
],
590+
)
591+
def test_check_minimum_version(monkeypatch, version, min_version, expected_pass):
592+
class TestIntegration(Integration):
593+
identifier = "test"
594+
595+
monkeypatch.setattr(sentry_sdk.integrations, "_MIN_VERSIONS", {"test": min_version})
596+
try:
597+
_check_minimum_version(TestIntegration, parse_version(version))
598+
except DidNotEnable:
599+
if expected_pass:
600+
assert False, (
601+
"_check_minimum_version raised DidNotEnable when it shouldn't have"
602+
)
603+
else:
604+
if not expected_pass:
605+
assert False, (
606+
"_check_minimum_version didn't raise DidNotEnable when it was supposed to"
607+
)
608+
609+
559610
@pytest.fixture
560611
def mock_client_with_dsn_netloc():
561612
"""

0 commit comments

Comments
 (0)