Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CDF-23588] 🧑‍💼 Env warning #1328

Merged
merged 18 commits into from
Jan 6, 2025
Merged
4 changes: 4 additions & 0 deletions CHANGELOG.cdf-tk.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,18 @@ Changes are grouped as follows:
with the flag `--offline`.
- [alpha feature] New subcommand `cdf modules pull` which pulls the configuration from the CDF project to the local
modules directory.
- Support for property `dataModelingType` in `LocationFilter` resources.

### Fixed

- No more warning about missing `.env` file when running in `Google Cloud Build`.
- No more warning about missing `.env` file if all variables are set in the environment.
- When deploying a `Sequence` resource, Cognite Toolkit now replaces `dataSetExternalId` with `dataSetId`.
- Cognite Toolkit has improved resources that have server set default values that can lead to redeploy even when
unchanged. This includes `Sequences`, `Containers`, `DataSets`, `Views`, `Nodes`, `Edges`, `ExtractionPipelines`,
`CogniteFiles`, `HostedExtractorJobs`, `Relationships`, `RobotMaps`, and `WorkflowVersions`.
- `LocationFilters` now parses the `version` key of `View` and `DataModel` correctly as a string.
- `LocationFilters` now converts an empty string of `dataSetExternalId` to `0` instead of ignoring it.

## [0.3.23] - 2024-12-13

Expand Down
5 changes: 4 additions & 1 deletion cognite_toolkit/_cdf_tk/apps/_core_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from cognite_toolkit._cdf_tk.feature_flags import Flags
from cognite_toolkit._cdf_tk.loaders import LOADER_BY_FOLDER_NAME
from cognite_toolkit._cdf_tk.utils import CDFToolConfig, get_cicd_environment
from cognite_toolkit._cdf_tk.utils.auth import AuthVariables
from cognite_toolkit._version import __version__ as current_version


Expand Down Expand Up @@ -115,7 +116,9 @@ def common(
if not (dotenv_file := Path.cwd() / ".env").is_file():
if not (dotenv_file := Path.cwd().parent / ".env").is_file():
if get_cicd_environment() == "local":
print("[bold yellow]WARNING:[/] No .env file found in current or parent directory.")
auth_vars = AuthVariables.from_env()
if not auth_vars.is_complete:
print("[bold yellow]WARNING:[/] No .env file found in current or parent directory.")

if dotenv_file.is_file():
has_loaded = load_dotenv(dotenv_file, override=override_env)
Expand Down
47 changes: 36 additions & 11 deletions cognite_toolkit/_cdf_tk/client/api/lookup.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,22 @@ def resource_name(self) -> str:
return type(self).__name__.removesuffix("LookUpAPI")

@overload
def id(self, external_id: str, is_dry_run: bool = False) -> int: ...
def id(self, external_id: str, is_dry_run: bool = False, allow_empty: bool = False) -> int: ...

@overload
def id(self, external_id: SequenceNotStr[str], is_dry_run: bool = False) -> list[int]: ...
def id(
self, external_id: SequenceNotStr[str], is_dry_run: bool = False, allow_empty: bool = False
) -> list[int]: ...

def id(self, external_id: str | SequenceNotStr[str], is_dry_run: bool = False) -> int | list[int]:
def id(
self, external_id: str | SequenceNotStr[str], is_dry_run: bool = False, allow_empty: bool = False
) -> int | list[int]:
ids = [external_id] if isinstance(external_id, str) else external_id
missing = [id for id in ids if id not in self._cache]
if allow_empty and "" in missing:
# Note we do not want to put empty string in the cache. It is a special case that
# as of 01/02/2025 only applies to LocationFilters
missing.remove("")
if missing:
try:
lookup = self._id(missing)
Expand All @@ -63,14 +71,19 @@ def id(self, external_id: str | SequenceNotStr[str], is_dry_run: bool = False) -
raise ResourceRetrievalError(
f"Failed to retrieve {self.resource_name} with external_id {missing}." "Have you created it?"
)
if is_dry_run:
return (
self._cache.get(external_id, self.dry_run_id)
if isinstance(external_id, str)
else [self._cache.get(id, self.dry_run_id) for id in ids]
)
return (
self._get_id_from_cache(external_id, is_dry_run, allow_empty)
if isinstance(external_id, str)
else [self._get_id_from_cache(id, is_dry_run, allow_empty) for id in ids]
)

return self._cache[external_id] if isinstance(external_id, str) else [self._cache[id] for id in ids]
def _get_id_from_cache(self, external_id: str, is_dry_run: bool = False, allow_empty: bool = False) -> int:
if allow_empty and external_id == "":
return 0
elif is_dry_run:
return self._cache.get(external_id, self.dry_run_id)
else:
return self._cache[external_id]

@overload
def external_id(self, id: int) -> str: ...
Expand All @@ -84,6 +97,8 @@ def external_id(
) -> str | list[str]:
ids = [id] if isinstance(id, int) else id
missing = [id_ for id_ in ids if id_ not in self._reverse_cache]
if 0 in missing:
missing.remove(0)
if missing:
try:
lookup = self._external_id(missing)
Expand All @@ -103,7 +118,17 @@ def external_id(
raise ResourceRetrievalError(
f"Failed to retrieve {self.resource_name} with id {missing}." "Have you created it?"
)
return self._reverse_cache[id] if isinstance(id, int) else [self._reverse_cache[id] for id in ids]
return (
self._get_external_id_from_cache(id)
if isinstance(id, int)
else [self._get_external_id_from_cache(id) for id in ids]
)

def _get_external_id_from_cache(self, id: int) -> str:
if id == 0:
# Reverse of looking up an empty string.
return ""
return self._reverse_cache[id]

@abstractmethod
def _id(self, external_id: SequenceNotStr[str]) -> dict[str, int]:
Expand Down
17 changes: 16 additions & 1 deletion cognite_toolkit/_cdf_tk/client/data_classes/location_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ def __init__(
scene: LocationFilterScene | None = None,
asset_centric: AssetCentricFilter | None = None,
views: list[LocationFilterView] | None = None,
data_modeling_type: Literal["HYBRID", "DATA_MODELING_ONLY"] | None = None,
) -> None:
self.external_id = external_id
self.name = name
Expand All @@ -158,6 +159,7 @@ def __init__(
self.scene = scene
self.asset_centric = asset_centric
self.views = views
self.data_modeling_type = data_modeling_type

def as_write(self) -> LocationFilterWrite:
return LocationFilterWrite(
Expand All @@ -170,6 +172,7 @@ def as_write(self) -> LocationFilterWrite:
scene=self.scene,
asset_centric=self.asset_centric,
views=self.views,
data_modeling_type=self.data_modeling_type,
)

def dump(self, camel_case: bool = True) -> dict[str, Any]:
Expand Down Expand Up @@ -208,6 +211,7 @@ def _load(cls, resource: dict[str, Any], cognite_client: CogniteClient | None =
scene=scene,
asset_centric=asset_centric,
views=views,
data_modeling_type=resource.get("dataModelingType"),
)


Expand Down Expand Up @@ -242,9 +246,19 @@ def __init__(
scene: LocationFilterScene | None = None,
asset_centric: AssetCentricFilter | None = None,
views: list[LocationFilterView] | None = None,
data_modeling_type: Literal["HYBRID", "DATA_MODELING_ONLY"] | None = None,
) -> None:
super().__init__(
external_id, name, parent_id, description, data_models, instance_spaces, scene, asset_centric, views
external_id,
name,
parent_id,
description,
data_models,
instance_spaces,
scene,
asset_centric,
views,
data_modeling_type,
)
self.id = id
self.created_time = created_time
Expand All @@ -267,6 +281,7 @@ def _load(cls, resource: dict[str, Any], cognite_client: CogniteClient | None =
views=[LocationFilterView._load(view) for view in resource["views"]] if "views" in resource else None,
created_time=resource["createdTime"],
updated_time=resource["lastUpdatedTime"],
data_modeling_type=resource.get("dataModelingType"),
)


Expand Down
7 changes: 6 additions & 1 deletion cognite_toolkit/_cdf_tk/commands/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
DataModelLoader,
ExtractionPipelineConfigLoader,
FileLoader,
LocationFilterLoader,
NodeLoader,
RawDatabaseLoader,
RawTableLoader,
Expand Down Expand Up @@ -455,7 +456,11 @@ def _replace_variables(
source_files.append(BuildSourceFile(source, content, None))
continue

if resource_name in {TransformationLoader.folder_name, DataModelLoader.folder_name}:
if resource_name in {
TransformationLoader.folder_name,
DataModelLoader.folder_name,
LocationFilterLoader.folder_name,
}:
# Ensure that all keys that are version gets read as strings.
# This is required by DataModels, Views, and Transformations that reference DataModels and Views.
content = quote_int_value_by_key_in_yaml(content, key="version")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from collections.abc import Hashable, Iterable, Sequence
from functools import lru_cache
from pathlib import Path
from typing import Any, final

from cognite.client.data_classes.capabilities import Capability, LocationFiltersAcl
Expand All @@ -16,7 +17,7 @@
LocationFilterWriteList,
)
from cognite_toolkit._cdf_tk.loaders._base_loaders import ResourceLoader
from cognite_toolkit._cdf_tk.utils import in_dict
from cognite_toolkit._cdf_tk.utils import in_dict, quote_int_value_by_key_in_yaml, safe_read
from cognite_toolkit._cdf_tk.utils.diff_list import diff_list_hashable, diff_list_identifiable, dm_identifier

from .classic_loaders import AssetLoader, SequenceLoader
Expand Down Expand Up @@ -90,19 +91,31 @@ def get_id(cls, item: LocationFilter | LocationFilterWrite | dict) -> str:
def dump_id(cls, id: str) -> dict[str, Any]:
return {"externalId": id}

def safe_read(self, filepath: Path | str) -> str:
# The version is a string, but the user often writes it as an int.
# YAML will then parse it as an int, for example, `3_0_2` will be parsed as `302`.
# This is technically a user mistake, as you should quote the version in the YAML file.
# However, we do not want to put this burden on the user (knowing the intricate workings of YAML),
# so we fix it here.
return quote_int_value_by_key_in_yaml(safe_read(filepath), key="version")

def load_resource(self, resource: dict[str, Any], is_dry_run: bool = False) -> LocationFilterWrite:
if parent_external_id := resource.pop("parentExternalId", None):
resource["parentId"] = self.client.lookup.location_filters.id(parent_external_id, is_dry_run)
if "assetCentric" not in resource:
return LocationFilterWrite._load(resource)
asset_centric = resource["assetCentric"]
if data_set_external_ids := asset_centric.pop("dataSetExternalIds", None):
asset_centric["dataSetIds"] = self.client.lookup.data_sets.id(data_set_external_ids, is_dry_run)
asset_centric["dataSetIds"] = self.client.lookup.data_sets.id(
data_set_external_ids, is_dry_run, allow_empty=True
)
for subfilter_name in self.subfilter_names:
subfilter = asset_centric.get(subfilter_name, {})
if data_set_external_ids := subfilter.pop("dataSetExternalIds", []):
asset_centric[subfilter_name]["dataSetIds"] = self.client.lookup.data_sets.id(
data_set_external_ids, is_dry_run
data_set_external_ids,
is_dry_run,
allow_empty=True,
)

return LocationFilterWrite._load(resource)
Expand All @@ -111,6 +124,9 @@ def dump_resource(self, resource: LocationFilter, local: dict[str, Any]) -> dict
dumped = resource.as_write().dump()
if parent_id := dumped.pop("parentId", None):
dumped["parentExternalId"] = self.client.lookup.location_filters.external_id(parent_id)
if "dataModelingType" in dumped and "dataModelingType" not in local:
# Default set on server side
dumped.pop("dataModelingType")
if "assetCentric" not in dumped:
return dumped
asset_centric = dumped["assetCentric"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@ description: LocationFilter for DataModel
dataModels:
- space: DataModelSpace
externalId: DataModelLocation
version: v1
version: '1_0_0'
assetCentric:
dataSetExternalIds:
- ''
6 changes: 5 additions & 1 deletion tests/test_integration/test_toolkit_client/test_locations.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,17 @@ def my_data_model(toolkit_client: ToolkitClient, toolkit_space: Space) -> DataMo
class TestLocationFilterAPI:
def test_create_retrieve_delete(self, toolkit_client: ToolkitClient, my_data_model: DataModel) -> None:
location_filter = LocationFilterWrite(
name="loc", external_id=SESSION_EXTERNAL_ID, data_models=[my_data_model.as_id()]
name="loc",
external_id=SESSION_EXTERNAL_ID,
data_models=[my_data_model.as_id()],
data_modeling_type="DATA_MODELING_ONLY",
)
created: LocationFilter | None = None
try:
created = toolkit_client.location_filters.create(location_filter)
assert isinstance(created, LocationFilter)
assert created.as_write().dump() == location_filter.dump()
assert created.data_modeling_type is not None

retrieved = toolkit_client.location_filters.retrieve(created.id)
assert isinstance(retrieved, LocationFilter)
Expand Down
12 changes: 8 additions & 4 deletions tests/test_unit/approval_client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ def __init__(self):
self._reverse_cache: dict[int, str] = {}

@staticmethod
def _create_id(string: str) -> int:
def _create_id(string: str, allow_empty: bool = False) -> int:
if allow_empty and string == "":
return 0
# This simulates CDF setting the internal ID.
# By using hashing, we will always get the same ID for the same string.
# Thus, the ID will be consistent between runs and can be used in snapshots.
Expand All @@ -95,15 +97,17 @@ def _create_id(string: str) -> int:
hash_int = int(hex_dig[:16], 16)
return hash_int

def id(self, external_id: str | SequenceNotStr[str], is_dry_run: bool = False) -> int | list[int]:
def id(
self, external_id: str | SequenceNotStr[str], is_dry_run: bool = False, allow_empty: bool = False
) -> int | list[int]:
if isinstance(external_id, str):
id_ = self._create_id(external_id)
id_ = self._create_id(external_id, allow_empty)
if id_ not in self._reverse_cache:
self._reverse_cache[id_] = external_id
return id_
output: list[int] = []
for ext_id in external_id:
id_ = self._create_id(ext_id)
id_ = self._create_id(ext_id, allow_empty)
if id_ not in self._reverse_cache:
self._reverse_cache[id_] = ext_id
output.append(id_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,13 @@ Location:
externalId: waterTreatmentPlant1_Windows_3_11_8
name: Water treatment plant
LocationFilter:
- dataModels:
- assetCentric:
dataSetIds:
- 0
dataModels:
- externalId: DataModelLocation
space: DataModelSpace
version: v1
version: '1_0_0'
description: LocationFilter for DataModel
externalId: DataModelLocationFilter
name: LocationFilter for DataModel
Expand Down
Loading