From 6529464dcb1068828770a31f1603c8b73b313dc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20V=2E=20Treider?= Date: Mon, 4 Dec 2023 11:04:01 +0100 Subject: [PATCH] post-v7 improvements & fixes (#1533) --- .github/workflows/build.yml | 3 --- .github/workflows/release.yaml | 22 ++++++++----------- cognite/client/data_classes/capabilities.py | 18 ++++++++++----- cognite/client/data_classes/iam.py | 9 +++++--- tests/tests_unit/test_api_client.py | 2 ++ .../test_data_classes/test_capabilities.py | 10 ++++----- .../test_data_classes/test_groups.py | 5 +++++ 7 files changed, 40 insertions(+), 29 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 160cc96fec..abf48007ab 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -70,8 +70,6 @@ jobs: poetry install - name: Test core - env: - CI: 1 run: pytest tests/tests_unit -n8 --dist loadscope --maxfail 10 -m 'not dsl' --test-deps-only-core @@ -103,7 +101,6 @@ jobs: COGNITE_PROJECT: python-sdk-test COGNITE_BASE_URL: https://greenfield.cognitedata.com COGNITE_CLIENT_NAME: python-sdk-integration-tests - CI: 1 run: | pytest tests --durations=10 --cov --cov-report xml:coverage.xml -n8 --dist loadscope --reruns 2 diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index da3a953835..31f7cfee32 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -24,8 +24,7 @@ jobs: poetry install -E numpy - name: Linting and static code checks - run: | - pre-commit run --all-files + run: pre-commit run --all-files - name: Verify proto files env: @@ -53,8 +52,6 @@ jobs: poetry install - name: Test core - env: - CI: 1 run: pytest tests/tests_unit -n8 --dist loadscope --maxfail 10 -m 'not dsl' --test-deps-only-core @@ -86,9 +83,7 @@ jobs: COGNITE_PROJECT: python-sdk-test COGNITE_BASE_URL: https://greenfield.cognitedata.com COGNITE_CLIENT_NAME: python-sdk-integration-tests - CI: 1 - run: | - pytest tests --cov --cov-report xml:coverage.xml -n8 --dist loadscope --reruns 2 + run: pytest tests --cov --cov-report xml:coverage.xml -n8 --dist loadscope --reruns 2 --maxfail 20 - uses: codecov/codecov-action@v3 with: @@ -97,7 +92,7 @@ jobs: build: runs-on: ubuntu-latest - needs: [lint, test_full] + needs: [lint, test_core, test_full] environment: CD steps: - uses: actions/checkout@v4 @@ -123,8 +118,9 @@ jobs: TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }} run: twine upload --verbose dist/* || echo 'Version exists' - - name: Push code snippets to service-contracts - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: sh ./scripts/deploy_code_snippets.sh || echo 'PR failed. There is probably - nothing to commit' + # TODO: Make this work again + # - name: Push code snippets to service-contracts + # env: + # GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # run: sh ./scripts/deploy_code_snippets.sh || echo 'PR failed. There is probably + # nothing to commit' diff --git a/cognite/client/data_classes/capabilities.py b/cognite/client/data_classes/capabilities.py index 68c5a4d79b..2551415cbd 100644 --- a/cognite/client/data_classes/capabilities.py +++ b/cognite/client/data_classes/capabilities.py @@ -35,12 +35,21 @@ class Capability(ABC): def __post_init__(self) -> None: if (capability_cls := type(self)) is UnknownAcl: return - acl = capability_cls.__name__ + try: + # There are so many things that may fail validation; non-enum passed, not iterable etc. + # We always want to show the example usage to the user. + self._validate() + except Exception as err: + raise ValueError( + f"Could not instantiate {capability_cls.__name__} due to: {err}. " + self.show_example_usage() + ) from err + + def _validate(self) -> None: + acl = (capability_cls := type(self)).__name__ if bad_actions := [a for a in self.actions if a not in capability_cls.Action]: full_action_examples = ", ".join(f"{acl}.Action.{action.name}" for action in capability_cls.Action) raise ValueError( - f"{acl} got unknown action(s): {bad_actions}, expected a subset of: [{full_action_examples}]. " - + self.show_example_usage() + f"{acl} got unknown action(s): {bad_actions}, expected a subset of: [{full_action_examples}]." ) allowed_scopes, scope_names = _VALID_SCOPES_BY_CAPABILITY[capability_cls] if type(self.scope) in allowed_scopes or not allowed_scopes: @@ -52,8 +61,7 @@ def __post_init__(self) -> None: else: full_scope_examples = ", ".join(f"{acl}.Scope.{name}" for name in scope_names) raise ValueError( - f"{acl} got an unknown scope: {self.scope}, expected an instance of one of: [{full_scope_examples}]. " - + self.show_example_usage() + f"{acl} got an unknown scope: {self.scope}, expected an instance of one of: [{full_scope_examples}]." ) @classmethod diff --git a/cognite/client/data_classes/iam.py b/cognite/client/data_classes/iam.py index ee9ead3a6c..ca62a68767 100644 --- a/cognite/client/data_classes/iam.py +++ b/cognite/client/data_classes/iam.py @@ -20,11 +20,12 @@ class Group(CogniteResource): Args: name (str): Name of the group source_id (str | None): ID of the group in the source. If this is the same ID as a group in the IDP, a service account in that group will implicitly be a part of this group as well. - capabilities (list[Capability] | None): No description. - id (int | None): No description. + capabilities (list[Capability] | None): List of capabilities (acls) this group should grant its users. + id (int | None): A server-generated ID for the object. is_deleted (bool | None): No description. deleted_time (int | None): No description. - metadata (dict[str, Any] | None): Custom, immutable application specific metadata. String key -> String value. Limits: + metadata (dict[str, Any] | None): Custom, immutable application specific metadata. String key -> String value. + Limits: Key are at most 32 bytes. Values are at most 512 bytes. Up to 16 key-value pairs. Total size is at most 4096. cognite_client (CogniteClient | None): The client to associate with this object. """ @@ -42,6 +43,8 @@ def __init__( self.name = name self.source_id = source_id self.capabilities = capabilities + if isinstance(self.capabilities, Capability): + self.capabilities = [capabilities] self.id = id self.is_deleted = is_deleted self.deleted_time = deleted_time diff --git a/tests/tests_unit/test_api_client.py b/tests/tests_unit/test_api_client.py index 8275192392..aed920e74b 100644 --- a/tests/tests_unit/test_api_client.py +++ b/tests/tests_unit/test_api_client.py @@ -3,6 +3,7 @@ import json import math import random +import time import unittest from collections import namedtuple from typing import Any, ClassVar, Literal, cast @@ -495,6 +496,7 @@ def request_callback(request): if int(np) == 3: return 503, {}, json.dumps({"message": "Service Unavailable"}) else: + time.sleep(0.001) # ensures bad luck race condition where 503 above executes last return 200, {}, json.dumps({"items": [{"x": 42, "y": 13}]}) rsps.add_callback( diff --git a/tests/tests_unit/test_data_classes/test_capabilities.py b/tests/tests_unit/test_data_classes/test_capabilities.py index 3839ff9d25..13d53b0a1c 100644 --- a/tests/tests_unit/test_data_classes/test_capabilities.py +++ b/tests/tests_unit/test_data_classes/test_capabilities.py @@ -249,13 +249,13 @@ def test_create_capability_forget_initializing_scope(self): assert grp1.dump() == grp2.dump() def test_create_capability_forget_initializing_scope__not_supported(self): - with pytest.raises(ValueError, match="^DataSetsAcl got an unknown scope"): + with pytest.raises(ValueError, match="DataSetsAcl got an unknown scope"): DataSetsAcl(actions=[DataSetsAcl.Action.Read], scope=CurrentUserScope) - with pytest.raises(ValueError, match="^DataSetsAcl got an unknown scope"): + with pytest.raises(ValueError, match="DataSetsAcl got an unknown scope"): DataSetsAcl(actions=[DataSetsAcl.Action.Read], scope=GroupsAcl.Scope.CurrentUser) - with pytest.raises(ValueError, match="^ExperimentsAcl got an unknown scope"): + with pytest.raises(ValueError, match="ExperimentsAcl got an unknown scope"): ExperimentsAcl(actions=[ExperimentsAcl.Action.Use], scope=AllScope) @@ -412,8 +412,8 @@ def test_idscopes_camel_case(): with pytest.raises(ValueError) as err: Capability.load(dct) assert err.value.args[0].startswith( - "DataSetsAcl got an unknown scope: IDScopeLowerCase(ids=[2495]), expected an instance of one of: " - "[DataSetsAcl.Scope.All, DataSetsAcl.Scope.ID]" + "Could not instantiate DataSetsAcl due to: DataSetsAcl got an unknown scope: IDScopeLowerCase(ids=[2495]), " + "expected an instance of one of: [DataSetsAcl.Scope.All, DataSetsAcl.Scope.ID]" ) diff --git a/tests/tests_unit/test_data_classes/test_groups.py b/tests/tests_unit/test_data_classes/test_groups.py index 8ad5ef2518..48925fac99 100644 --- a/tests/tests_unit/test_data_classes/test_groups.py +++ b/tests/tests_unit/test_data_classes/test_groups.py @@ -5,6 +5,7 @@ import pytest from cognite.client.data_classes import Group, GroupList +from cognite.client.data_classes.capabilities import DataModelInstancesAcl def raw_groups(): @@ -39,6 +40,10 @@ def raw_groups(): class TestGroups: + def test_group_init__accept_single_acl(self) -> None: + acl = DataModelInstancesAcl([DataModelInstancesAcl.Action.Write], DataModelInstancesAcl.Scope.All()) + assert Group(name="a", capabilities=acl) == Group(name="a", capabilities=[acl]) + @pytest.mark.parametrize("raw", list(raw_groups())) def test_load_dump_unknown_group(self, raw: dict[str, Any]) -> None: group = Group.load(raw)