NAS-5360 Enforcing api-token permission python sdk update #398

JKlucka · 2023-10-30T06:55:43Z

No description provided.

ghost · 2023-10-30T06:55:50Z

Build succeeded (check pipeline).

codecov-commenter · 2023-10-30T06:59:02Z

Codecov Report

Merging #398 (00aa6db) into master (c3772ac) will increase coverage by 0.15%.
Report is 30 commits behind head on master.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master     #398      +/-   ##
==========================================
+ Coverage   91.19%   91.34%   +0.15%     
==========================================
  Files          90       91       +1     
  Lines        6005     6091      +86     
==========================================
+ Hits         5476     5564      +88     
+ Misses        529      527       -2

Files	Coverage Δ
gooddata-sdk/gooddata_sdk/__init__.py	`100.00% <ø> (ø)`
...mission/declarative_model/dashboard_permissions.py	`89.36% <ø> (-0.12%)`	⬇️
.../declarative_model/manage_dashboard_permissions.py	`100.00% <ø> (+5.26%)`	⬆️
...catalog/permission/declarative_model/permission.py	`98.57% <ø> (-1.43%)`	⬇️
...ata-sdk/gooddata_sdk/catalog/permission/service.py	`100.00% <ø> (ø)`
gooddata-sdk/gooddata_sdk/catalog/rule.py	`100.00% <ø> (ø)`
gooddata-sdk/gooddata_sdk/catalog/setting.py	`90.90% <ø> (+0.43%)`	⬆️
...model/workspace/analytics_model/analytics_model.py	`97.86% <ø> (-0.47%)`	⬇️
.../entity_model/content_objects/workspace_setting.py	`93.54% <ø> (+1.24%)`	⬆️
...catalog/workspace/entity_model/user_data_filter.py	`74.80% <ø> (ø)`
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

ghost · 2023-10-30T07:18:33Z

Build succeeded (check pipeline).

gooddata-sdk/gooddata_sdk/catalog/permission/service.py

gooddata-sdk/tests/catalog/fixtures/permissions/manage_organization_permissions.yaml

ghost · 2023-10-30T09:20:05Z

Build succeeded (check pipeline).

ghost · 2023-10-30T10:50:49Z

Build succeeded (check pipeline).

ghost · 2023-10-30T11:51:24Z

Build succeeded (check pipeline).

hkad98 · 2023-10-30T13:54:26Z

gooddata-sdk/tests/catalog/test_catalog_permission.py

@@ -54,6 +56,12 @@ def _assert_default_permissions(catalog_declarative_permissions: CatalogDeclarat
    assert set(permission.name for permission in catalog_declarative_permissions.permissions) == {"ANALYZE", "VIEW"}


+def _assert_organization_permissions_id(
+    catalog_organization_permissions: [CatalogDeclarativeOrganizationPermission],


This is not correct typing. Note that mypy does not work in tests, so it is undetected. Please use List[CatalogDeclarativeOrganizationPermission].

I missed this one, thank you

hkad98 · 2023-10-30T13:56:57Z

gooddata-sdk/gooddata_sdk/catalog/permission/service.py

+        """
+
+        catalog_list = []
+        for permission in self._layout_api.get_organization_permissions():


Please store the result from self._layout_api.get_organization_permissions() first and then use it.

e.g.:
for permission in organization_permissions:

updated as recommended

hkad98 · 2023-10-30T13:58:01Z

gooddata-sdk/gooddata_sdk/catalog/permission/service.py

+            None
+        """
+        permissions = [permission.to_api() for permission in organization_permission_assignments]
+        print(permissions)


Please delete this print.

forgot to remove, thanks

hkad98 · 2023-10-30T13:58:28Z

gooddata-sdk/gooddata_sdk/catalog/permission/service.py

+        Returns:
+            None
+        """
+        permissions = [permission.to_api() for permission in organization_permission_assignments]


This seems to be unused.

right, now to be used as input param for the following line:
self._actions_api.manage_organization_permissions(permissions, _check_return_type=False,)

hkad98 · 2023-10-30T14:38:13Z

gooddata-sdk/tests/catalog/test_catalog_permission.py

+
+
+@gd_vcr.use_cassette(str(_fixtures_dir / "put_declarative_organization_permissions.yaml"))
+def test_put_and_get_declarative_organization_permissions(test_config):
+    expected_json_path = _current_dir / "expected" / "declarative_organization_permissions.json"
+    sdk = GoodDataSdk.create(host_=test_config["host"], token_=test_config["token"])
+    catalog_declarative_permissions_initial = sdk.catalog_permission.get_declarative_organization_permissions()
+
+    assert len(catalog_declarative_permissions_initial) == 1
+    _assert_organization_permissions_id(catalog_declarative_permissions_initial)
+    assert set(org_permission.name for org_permission in catalog_declarative_permissions_initial) == {"MANAGE"}
+
+    with open(expected_json_path, "r", encoding="utf-8") as f:
+        data = json.load(f)
+
+    declarative_organization_permissions = []
+    for permission in data:
+        declarative_organization_permissions.append(CatalogDeclarativeOrganizationPermission.from_api(permission))
+
+    sdk.catalog_permission.put_declarative_organization_permissions(declarative_organization_permissions)
+
+    catalog_declarative_permissions_after_put = sdk.catalog_permission.get_declarative_organization_permissions()
+    assert len(catalog_declarative_permissions_after_put) == 2
+    _assert_organization_permissions_id(catalog_declarative_permissions_after_put)
+    assert set(org_permission.name for org_permission in catalog_declarative_permissions_after_put) == {
+        "MANAGE",
+        "SELF_CREATE_TOKEN",
+    }
+
+
+@gd_vcr.use_cassette(str(_fixtures_dir / "manage_organization_permissions.yaml"))
+def test_manage_organization_permissions(test_config):
+    sdk = GoodDataSdk.create(host_=test_config["host"], token_=test_config["token"])
+
+    # assign permissions to adminGroup
+    sdk.catalog_permission.manage_organization_permissions(
+        [
+            CatalogOrganizationPermissionAssignment(
+                assignee_identifier=CatalogAssigneeIdentifier(id="adminGroup", type="userGroup"),
+                permissions=["MANAGE", "SELF_CREATE_TOKEN"],
+            )
+        ],
+    )
+
+    catalog_declarative_permissions_initial = sdk.catalog_permission.get_declarative_organization_permissions()
+    assert len(catalog_declarative_permissions_initial) == 2
+    assert set(org_permission.name for org_permission in catalog_declarative_permissions_initial) == {
+        "MANAGE",
+        "SELF_CREATE_TOKEN",
+    }


I am still determining the tests. There is no AfterAll or AfterEach mechanism as is, for example, known in Java. Therefore, if you put something, then it stays put. There should be a try-finally block where the change is reverted in the final block back to the original state.

updated as discussed. Thank you

ghost · 2023-10-30T16:36:30Z

Build succeeded (check pipeline).

ghost · 2023-10-30T16:37:34Z

Build succeeded (check pipeline).

yenkins · 2023-10-30T19:49:28Z

Sonar scan result

More detail, see in https://sonarqube-gate.intgdc.com/dashboard?id=gooddata-python-sdk-gate-PR398

To scan for vulnerabilities in dependencies and run unit tests (to get coverage report in sonar) please comment your PR with 'extended check sonar'.

ghost · 2023-10-30T19:49:44Z

Build succeeded (gate pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-python-sdk-fossa-scanning-zuul/214/ : FAILURE in 1m 21s (non-voting)
https://checklist.intgdc.com/job/compliance/job/gooddata-python-sdk-sonarqube-scan-zuul-gate/364/ : SUCCESS in 1m 10s
https://checklist.intgdc.com/job/compliance/job/gooddata-python-sdk-merge-compliance-check/368/ : SUCCESS in 10s

JKlucka requested review from lupko, pcerny, jaceksan and hkad98 as code owners October 30, 2023 06:55

JKlucka force-pushed the jklu/nas-5360-new branch from 82ce8d3 to 8f7546a Compare October 30, 2023 07:18

vojtechtoman reviewed Oct 30, 2023

View reviewed changes

gooddata-sdk/gooddata_sdk/catalog/permission/service.py Outdated Show resolved Hide resolved

gooddata-sdk/gooddata_sdk/catalog/permission/service.py Outdated Show resolved Hide resolved

gooddata-sdk/tests/catalog/fixtures/permissions/manage_organization_permissions.yaml Show resolved Hide resolved

JKlucka force-pushed the jklu/nas-5360-new branch from 8f7546a to 00aa6db Compare October 30, 2023 09:20

JKlucka force-pushed the jklu/nas-5360-new branch from 00aa6db to e498de2 Compare October 30, 2023 10:50

JKlucka force-pushed the jklu/nas-5360-new branch from e498de2 to f7e6b3a Compare October 30, 2023 11:51

hkad98 reviewed Oct 30, 2023

View reviewed changes

NAS-5360 python sdk client update

f9e5ae4

JKlucka force-pushed the jklu/nas-5360-new branch from 30981ca to f9e5ae4 Compare October 30, 2023 16:37

hkad98 approved these changes Oct 30, 2023

View reviewed changes

hkad98 added the merge label Oct 30, 2023

ghost removed the merge label Oct 30, 2023

ghost merged commit d705310 into gooddata:master Oct 30, 2023
6 checks passed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NAS-5360 Enforcing api-token permission python sdk update #398

NAS-5360 Enforcing api-token permission python sdk update #398

JKlucka commented Oct 30, 2023

ghost commented Oct 30, 2023

codecov-commenter commented Oct 30, 2023 •

edited

Loading

ghost commented Oct 30, 2023

ghost commented Oct 30, 2023

ghost commented Oct 30, 2023

ghost commented Oct 30, 2023

hkad98 Oct 30, 2023

JKlucka Oct 30, 2023

hkad98 Oct 30, 2023

JKlucka Oct 30, 2023

hkad98 Oct 30, 2023

JKlucka Oct 30, 2023

hkad98 Oct 30, 2023

JKlucka Oct 30, 2023

hkad98 Oct 30, 2023

JKlucka Oct 30, 2023

ghost commented Oct 30, 2023

ghost commented Oct 30, 2023

yenkins commented Oct 30, 2023

ghost commented Oct 30, 2023

NAS-5360 Enforcing api-token permission python sdk update #398

NAS-5360 Enforcing api-token permission python sdk update #398

Conversation

JKlucka commented Oct 30, 2023

ghost commented Oct 30, 2023

codecov-commenter commented Oct 30, 2023 • edited Loading

Codecov Report

ghost commented Oct 30, 2023

ghost commented Oct 30, 2023

ghost commented Oct 30, 2023

ghost commented Oct 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Oct 30, 2023

ghost commented Oct 30, 2023

yenkins commented Oct 30, 2023

Sonar scan result

ghost commented Oct 30, 2023

codecov-commenter commented Oct 30, 2023 •

edited

Loading