Skip to content

Commit

Permalink
fix: Passing empty string to external_id in datapoints.retrieve_dataf…
Browse files Browse the repository at this point in the history
…rame raises error (#2070)

Co-authored-by: Håkon V. Treider <[email protected]>
  • Loading branch information
MortGron and haakonvt authored Dec 20, 2024
1 parent bbe8f99 commit ba07876
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 10 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ Changes are grouped as follows
- `Fixed` for any bug fixes.
- `Security` in case of vulnerabilities.

## [7.70.7] - 2024-12-20
### Fixed
- Passing a valid but empty string as external_id no longer raises an error for certain SDK methods

## [7.70.6] - 2024-12-14
### Fixed
- Updating a Sequence and repeating existing columns no longer raises a `CogniteDuplicatedError`.
Expand Down
2 changes: 1 addition & 1 deletion cognite/client/_version.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from __future__ import annotations

__version__ = "7.70.6"
__version__ = "7.70.7"

__api_subversion__ = "20230101"
16 changes: 8 additions & 8 deletions cognite/client/utils/_identifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def as_primitive(self) -> str | int: ...
class Identifier(Generic[T_ID]):
def __init__(self, value: T_ID) -> None:
if not isinstance(value, (int, str, InstanceId)):
raise TypeError(f"Expected id/external_id to be of type int or str, got {value} of type {type(id)}")
raise TypeError(f"Expected id/external_id to be of type int or str, got {value} of type {type(value)}")
self.__value: T_ID = value

def __eq__(self, other: Any) -> bool:
Expand All @@ -86,16 +86,16 @@ def __repr__(self) -> str:

@classmethod
def of_either(
cls, id: int | None, external_id: str | None, instance_id: InstanceId | tuple[str, str] | None = None
cls, id_: int | None, external_id: str | None, instance_id: InstanceId | tuple[str, str] | None = None
) -> Identifier:
if id is external_id is instance_id is None:
if id_ is external_id is instance_id is None:
raise ValueError("Exactly one of id, external id, or instance_id must be specified, got neither")
elif id is not None:
elif id_ is not None:
if external_id is not None or instance_id is not None:
raise ValueError("Exactly one of id, external id, or instance_id must be specified, got multiple")
elif not isinstance(id, int):
raise TypeError(f"Invalid id, expected int, got {type(id)}")
elif not 1 <= id <= MAX_VALID_INTERNAL_ID:
elif not isinstance(id_, int):
raise TypeError(f"Invalid id, expected int, got {type(id_)}")
elif not 1 <= id_ <= MAX_VALID_INTERNAL_ID:
raise ValueError(f"Invalid id, must satisfy: 1 <= id <= {MAX_VALID_INTERNAL_ID}")
elif external_id is not None:
if instance_id is not None:
Expand All @@ -107,7 +107,7 @@ def of_either(
instance_id = InstanceId.load(instance_id)
if not isinstance(instance_id, InstanceId):
raise TypeError(f"Invalid instance_id, expected InstanceId, got {type(instance_id)}")
return Identifier(id or external_id or instance_id)
return Identifier(next(idx for idx in (id_, external_id, instance_id) if idx is not None))

@classmethod
def load(
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[tool.poetry]
name = "cognite-sdk"

version = "7.70.6"
version = "7.70.7"

description = "Cognite Python SDK"
readme = "README.md"
Expand Down
2 changes: 2 additions & 0 deletions tests/tests_unit/test_utils/test_identifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ class TestIdentifier:
(1, None, None, ("id", 1)),
(MAX_VALID_INTERNAL_ID, None, None, ("id", MAX_VALID_INTERNAL_ID)),
(None, "foo", None, ("external_id", "foo")),
# Bug prior to 7.70.7, empty string external ID would fail with wrong error message
(None, "", None, ("external_id", "")),
(None, None, InstanceId("s", "extid"), ("instance_id", InstanceId("s", "extid"))),
),
)
Expand Down

0 comments on commit ba07876

Please sign in to comment.