-
Notifications
You must be signed in to change notification settings - Fork 70
feat(gooddata-sdk): report whether create_or_update created or updated #1705
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,20 @@ | ||
| # (C) 2022 GoodData Corporation | ||
| from __future__ import annotations | ||
|
|
||
| from enum import Enum | ||
|
|
||
| # Use typing collection types to support python < py3.9 | ||
| ValidObjects = dict[str, set[str]] | ||
|
|
||
|
|
||
| class UpsertOutcome(str, Enum): | ||
| """Which branch a ``create_or_update*`` method took. | ||
|
|
||
| The outcome is best-effort: it reports the branch the SDK chose after its | ||
| existence check, and that check is not atomic with the write that follows. | ||
| A concurrent actor can create or delete the entity in between, so treat the | ||
| value as informational rather than as an authoritative audit record. | ||
| """ | ||
|
|
||
| CREATED = "created" | ||
| UPDATED = "updated" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| from gooddata_api_client.model.json_api_api_token_in_document import JsonApiApiTokenInDocument | ||
|
|
||
| from gooddata_sdk.catalog.catalog_service_base import CatalogServiceBase | ||
| from gooddata_sdk.catalog.types import UpsertOutcome | ||
| from gooddata_sdk.catalog.user.declarative_model.user import CatalogDeclarativeUsers | ||
| from gooddata_sdk.catalog.user.declarative_model.user_and_user_groups import CatalogDeclarativeUsersUserGroups | ||
| from gooddata_sdk.catalog.user.declarative_model.user_group import CatalogDeclarativeUserGroups | ||
|
|
@@ -25,7 +26,7 @@ | |
| class CatalogUserService(CatalogServiceBase): | ||
| # Entity methods for users | ||
|
|
||
| def create_or_update_user(self, user: CatalogUser) -> None: | ||
| def create_or_update_user(self, user: CatalogUser) -> UpsertOutcome: | ||
| """Creates a new user or overwrites an existing user. | ||
|
|
||
|
|
||
|
|
@@ -34,7 +35,8 @@ def create_or_update_user(self, user: CatalogUser) -> None: | |
| User entity object. | ||
|
|
||
| Returns: | ||
| None | ||
| UpsertOutcome: | ||
| CREATED if the user did not exist yet, UPDATED if it did. | ||
|
Comment on lines
37
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Document that The existence check and write are not atomic. Another actor can change the resource between these operations. State that the outcome reports the branch selected by the SDK. Do not describe it as an authoritative existence result.
📍 Affects 7 files
🤖 Prompt for AI Agents |
||
| """ | ||
| try: | ||
| self.get_user(user_id=user.id) | ||
|
|
@@ -43,6 +45,8 @@ def create_or_update_user(self, user: CatalogUser) -> None: | |
| except NotFoundException: | ||
| user_document = CatalogUserDocument(data=user) | ||
| self._entities_api.create_entity_users(json_api_user_in_document=user_document.to_api()) | ||
| return UpsertOutcome.CREATED | ||
| return UpsertOutcome.UPDATED | ||
|
|
||
| def get_user(self, user_id: str) -> CatalogUser: | ||
| """Get an individual user using User id. | ||
|
|
@@ -89,15 +93,16 @@ def list_users(self) -> list[CatalogUser]: | |
|
|
||
| # Entity methods for user groups | ||
|
|
||
| def create_or_update_user_group(self, user_group: CatalogUserGroup) -> None: | ||
| def create_or_update_user_group(self, user_group: CatalogUserGroup) -> UpsertOutcome: | ||
| """Create a new user group or overwrite an existing user group. | ||
|
|
||
| Args: | ||
| user_group (CatalogUserGroup): | ||
| UserGroup entity object. | ||
|
|
||
| Returns: | ||
| None | ||
| UpsertOutcome: | ||
| CREATED if the user group did not exist yet, UPDATED if it did. | ||
| """ | ||
| try: | ||
| self.get_user_group(user_group_id=user_group.id) | ||
|
|
@@ -108,6 +113,8 @@ def create_or_update_user_group(self, user_group: CatalogUserGroup) -> None: | |
| except NotFoundException: | ||
| user_group_document = CatalogUserGroupDocument(data=user_group) | ||
| self._entities_api.create_entity_user_groups(user_group_document.to_api()) | ||
| return UpsertOutcome.CREATED | ||
| return UpsertOutcome.UPDATED | ||
|
|
||
| def get_user_group(self, user_group_id: str) -> CatalogUserGroup: | ||
| """Get an individual user group using user group id. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we would like to eventually switch this to StrEnum – we cannot right now because we have a support for Python 3.10 which does not support StrEnum.
Consider adding the following:
It should make switch to StrEnum a noop.