Skip to content

Commit

Permalink
[CDF-23800] 👁️‍🗨️ Improved error messages (#1475)
Browse files Browse the repository at this point in the history
* refactor; stipulate solution

* refactor; finished scaffolding

* tests; added test for new case

* feat; improved error message
  • Loading branch information
doctrino authored Feb 24, 2025
1 parent 5c3146b commit 13ebbf5
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
SessionsAcl,
)
from cognite.client.data_classes.functions import HANDLER_FILE_NAME
from cognite.client.exceptions import CogniteAuthError
from cognite.client.utils.useful_types import SequenceNotStr
from rich import print
from rich.console import Console
Expand All @@ -42,7 +43,7 @@
calculate_secure_hash,
calculate_str_or_file_hash,
)
from cognite_toolkit._cdf_tk.utils.cdf import read_auth
from cognite_toolkit._cdf_tk.utils.cdf import read_auth, try_find_error

from .auth_loaders import GroupAllScopedLoader
from .data_organization_loaders import DataSetsLoader
Expand Down Expand Up @@ -481,7 +482,13 @@ def create(self, items: FunctionScheduleWriteList) -> FunctionSchedulesList:
if id_ not in self.authentication_by_id:
raise ToolkitRequiredValueError(f"Authentication is missing for schedule {id_!r}")
client_credentials = self.authentication_by_id[id_]
created = self.client.functions.schedules.create(item, client_credentials=client_credentials)
try:
created = self.client.functions.schedules.create(item, client_credentials=client_credentials)
except CogniteAuthError as e:
if hint := try_find_error(client_credentials):
raise ResourceCreationError(f"Failed to create Function Schedule {id_}: {hint}") from e
raise e

# The PySDK mutates the input object, such that function_id is set and function_external_id is None.
# If we call .get_id on the returned object, it will raise an error we require the function_external_id
# to be set.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from __future__ import annotations

import warnings
from collections import defaultdict
from collections.abc import Hashable, Iterable, Sequence
from functools import lru_cache
from pathlib import Path
Expand Down Expand Up @@ -57,13 +58,14 @@
TransformationNotificationWrite,
TransformationNotificationWriteList,
)
from cognite.client.exceptions import CogniteAPIError, CogniteDuplicatedError, CogniteNotFoundError
from cognite.client.exceptions import CogniteAPIError, CogniteAuthError, CogniteDuplicatedError, CogniteNotFoundError
from cognite.client.utils.useful_types import SequenceNotStr
from rich import print

from cognite_toolkit._cdf_tk._parameters import ANY_INT, ParameterSpec, ParameterSpecSet
from cognite_toolkit._cdf_tk.client.data_classes.raw import RawDatabase, RawTable
from cognite_toolkit._cdf_tk.exceptions import (
ResourceCreationError,
ToolkitFileNotFoundError,
ToolkitInvalidParameterNameError,
ToolkitRequiredValueError,
Expand All @@ -74,11 +76,13 @@
from cognite_toolkit._cdf_tk.loaders._base_loaders import ResourceLoader
from cognite_toolkit._cdf_tk.utils import (
calculate_secure_hash,
humanize_collection,
in_dict,
load_yaml_inject_variables,
quote_int_value_by_key_in_yaml,
safe_read,
)
from cognite_toolkit._cdf_tk.utils.cdf import try_find_error
from cognite_toolkit._cdf_tk.utils.diff_list import diff_list_hashable

from .auth_loaders import GroupAllScopedLoader
Expand Down Expand Up @@ -321,20 +325,49 @@ def create(self, items: Sequence[TransformationWrite]) -> TransformationList:
warnings.simplefilter("ignore")
# Ignoring warnings from SDK about session unauthorized. Motivation is CDF is not fast enough to
# handle first a group that authorizes the session and then the transformation.
return self.client.transformations.create(items)
try:
return self.client.transformations.create(items)
except CogniteAuthError as e:
if error := self._create_auth_creation_error(items):
raise error from e
raise e

def retrieve(self, ids: SequenceNotStr[str | int]) -> TransformationList:
internal_ids, external_ids = self._split_ids(ids)
return self.client.transformations.retrieve_multiple(
ids=internal_ids, external_ids=external_ids, ignore_unknown_ids=True
)

def update(self, items: TransformationWriteList) -> TransformationList:
def update(self, items: Sequence[TransformationWrite]) -> TransformationList:
with warnings.catch_warnings():
warnings.simplefilter("ignore")
# Ignoring warnings from SDK about session unauthorized. Motivation is CDF is not fast enough to
# handle first a group that authorizes the session and then the transformation.
return self.client.transformations.update(items, mode="replace")
try:
return self.client.transformations.update(items, mode="replace")
except CogniteAuthError as e:
if error := self._create_auth_creation_error(items):
raise error from e
raise e

@staticmethod
def _create_auth_creation_error(items: Sequence[TransformationWrite]) -> ResourceCreationError | None:
hints_by_id: dict[str, list[str]] = defaultdict(list)
for item in items:
if not item.external_id:
continue
hint_source = try_find_error(item.source_oidc_credentials)
if hint_source:
hints_by_id[item.external_id].append(hint_source)
if hint_dest := try_find_error(item.destination_oidc_credentials):
if hint_dest != hint_source:
hints_by_id[item.external_id].append(hint_dest)
if hints_by_id:
body = "\n".join(f" {id_} - {humanize_collection(hints)}" for id_, hints in hints_by_id.items())
return ResourceCreationError(
f"Failed to create Transformation(s) du to likely invalid credentials:\n{body}",
)
return None

def delete(self, ids: SequenceNotStr[str | int]) -> int:
existing = self.retrieve(ids).as_ids()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,15 @@
Capability,
WorkflowOrchestrationAcl,
)
from cognite.client.exceptions import CogniteNotFoundError
from cognite.client.exceptions import CogniteAuthError, CogniteNotFoundError
from cognite.client.utils.useful_types import SequenceNotStr
from rich import print
from rich.console import Console

from cognite_toolkit._cdf_tk._parameters import ANY_INT, ANY_STR, ANYTHING, ParameterSpec, ParameterSpecSet
from cognite_toolkit._cdf_tk.client import ToolkitClient
from cognite_toolkit._cdf_tk.exceptions import (
ResourceCreationError,
ToolkitRequiredValueError,
)
from cognite_toolkit._cdf_tk.feature_flags import Flags
Expand All @@ -57,7 +58,7 @@
ToolkitWarning,
)
from cognite_toolkit._cdf_tk.utils import calculate_secure_hash, humanize_collection, to_directory_compatible
from cognite_toolkit._cdf_tk.utils.cdf import read_auth
from cognite_toolkit._cdf_tk.utils.cdf import read_auth, try_find_error
from cognite_toolkit._cdf_tk.utils.diff_list import diff_list_hashable, diff_list_identifiable

from .auth_loaders import GroupAllScopedLoader
Expand Down Expand Up @@ -491,10 +492,18 @@ def get_required_capability(
def create(self, items: WorkflowTriggerUpsertList) -> WorkflowTriggerList:
created = WorkflowTriggerList([])
for item in items:
credentials = self._authentication_by_id.get(item.external_id)
created.append(self.client.workflows.triggers.upsert(item, credentials))
created.append(self._create_item(item))
return created

def _create_item(self, item: WorkflowTriggerUpsert) -> WorkflowTrigger:
credentials = self._authentication_by_id.get(item.external_id)
try:
return self.client.workflows.triggers.upsert(item, credentials)
except CogniteAuthError as e:
if hint := try_find_error(credentials):
raise ResourceCreationError(f"Failed to create WorkflowTrigger {item.external_id}: {hint}") from e
raise e

def retrieve(self, ids: SequenceNotStr[str]) -> WorkflowTriggerList:
all_triggers = self.client.workflows.triggers.list(limit=-1)
lookup = set(ids)
Expand All @@ -508,8 +517,7 @@ def update(self, items: WorkflowTriggerUpsertList) -> WorkflowTriggerList:
if item.external_id in existing_lookup:
self.client.workflows.triggers.delete(external_id=item.external_id)

credentials = self._authentication_by_id.get(item.external_id)
created = self.client.workflows.triggers.upsert(item, client_credentials=credentials)
created = self._create_item(item)
updated.append(created)
return updated

Expand Down
26 changes: 26 additions & 0 deletions cognite_toolkit/_cdf_tk/utils/cdf.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
from collections.abc import Hashable, Iterator
from typing import Any, Literal, overload
from urllib.parse import urlparse

from cognite.client.credentials import OAuthClientCredentials
from cognite.client.data_classes import (
ClientCredentials,
OidcCredentials,
)
from cognite.client.data_classes.data_modeling import Edge, Node, ViewId
from cognite.client.data_classes.filters import SpaceFilter
from cognite.client.exceptions import CogniteAPIError
from rich.console import Console

from cognite_toolkit._cdf_tk.client import ToolkitClient
from cognite_toolkit._cdf_tk.constants import ENV_VAR_PATTERN
from cognite_toolkit._cdf_tk.exceptions import (
ToolkitRequiredValueError,
ToolkitTypeError,
Expand All @@ -20,6 +23,29 @@
HighSeverityWarning,
MediumSeverityWarning,
)
from cognite_toolkit._cdf_tk.utils import humanize_collection


def try_find_error(credentials: OidcCredentials | ClientCredentials | None) -> str | None:
if credentials is None:
return None
missing: list[str] = []
if client_id := ENV_VAR_PATTERN.match(credentials.client_id):
missing.append(client_id.group(1))
if client_secret := ENV_VAR_PATTERN.match(credentials.client_secret):
missing.append(client_secret.group(1))
if missing:
plural = "s are" if len(missing) > 1 else " is"
return f"The environment variable{plural} not set: {humanize_collection(missing)}."
if isinstance(credentials, ClientCredentials):
return None
try:
result = urlparse(credentials.token_uri)
if not all([result.scheme, result.netloc]):
raise ValueError
except ValueError:
return f"The tokenUri {credentials.token_uri!r} is not a valid URI."
return None


@overload
Expand Down
29 changes: 29 additions & 0 deletions tests/test_unit/test_cdf_tk/test_utils/test_cdf.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import pytest
from cognite.client.data_classes import ClientCredentials, OidcCredentials

from cognite_toolkit._cdf_tk.utils.cdf import try_find_error


class TestTryFindError:
@pytest.mark.parametrize(
"credentials, expected",
[
pytest.param(
ClientCredentials("${ENVIRONMENT_VAR}", "1234"),
"The environment variable is not set: ENVIRONMENT_VAR.",
id="Missing environment variable",
),
pytest.param(
ClientCredentials("${ENV1}", "${ENV2}"),
"The environment variables are not set: ENV1 and ENV2.",
id="Missing environment variable",
),
pytest.param(
OidcCredentials("my-client-id", "123", ["https://cognite.com"], "not-valid-uri", "my-project"),
"The tokenUri 'not-valid-uri' is not a valid URI.",
),
pytest.param(None, None, id="empty"),
],
)
def test_try_find_error(self, credentials: OidcCredentials | ClientCredentials | None, expected: str | None):
assert try_find_error(credentials) == expected

0 comments on commit 13ebbf5

Please sign in to comment.