Skip to content

Conversation

janmatzek
Copy link
Contributor

No description provided.

@janmatzek janmatzek force-pushed the SVS-1245-add-custom-fields-to-gooddata-pipelines branch from 3dcbaa4 to e4ec7a3 Compare September 12, 2025 14:45
@janmatzek janmatzek marked this pull request as ready for review September 15, 2025 05:16
@janmatzek janmatzek enabled auto-merge September 15, 2025 05:16
Comment on lines 215 to 224
def get_workspace_layout(self, workspace_id: str) -> requests.Response:
"""Get the layout of the specified workspace.
Args:
workspace_id (str): The ID of the workspace to retrieve the layout for.
Returns:
requests.Response: The response containing the workspace layout.
"""
endpoint = f"/layout/workspaces/{workspace_id}"
return self._get(endpoint)
Copy link
Contributor

@hkad98 hkad98 Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use get_declarative_workspace from gooddata_sdk?

Comment on lines 226 to 239
def put_workspace_layout(
self, workspace_id: str, layout: dict[str, Any]
) -> requests.Response:
"""Update the layout of the specified workspace.
Args:
workspace_id (str): The ID of the workspace to update.
layout (dict[str, Any]): The new layout to set for the workspace.
Returns:
requests.Response: The response from the server after updating the layout.
"""
endpoint = f"/layout/workspaces/{workspace_id}"
headers = {**self.headers, "Content-Type": "application/json"}
return self._put(endpoint, data=layout, headers=headers)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use put_declarative_workspace from gooddata_sdk?

Comment on lines 163 to 166
@staticmethod
def _set_contains(set_to_check: set[Any], item: Any) -> bool:
"""Helper function to check if an item is in a set."""
return item in set_to_check
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this method necessary? Why not simplify do the item in set_to_check?

Comment on lines 148 to 151
if set_new_invalid_relations.issubset(set_current_invalid_relations):
return True
else:
return False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if set_new_invalid_relations.issubset(set_current_invalid_relations):
return True
else:
return False
return set_new_invalid_relations.issubset(set_current_invalid_relations)

Comment on lines 98 to 108
metrics_response = self._api.get_all_metrics(workspace_id)
visualizations_response = self._api.get_all_visualization_objects(
workspace_id
)
dashboards_response = self._api.get_all_dashboards(workspace_id)
self._api.raise_if_response_not_ok(
metrics_response,
visualizations_response,
dashboards_response,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to use any methods from gooddata_sdk? This seems to me like re-implementing something that already exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. I need to use the "X-GDC-VALIDATE-RELATIONS": "true" header for these to get the areRelationsValid attribute. I didn't see a way to get that using the SDK.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see... maybe we could extend the gooddata_sdk to set custom headers somehow. Let's leave it for follow-up.

@janmatzek janmatzek force-pushed the SVS-1245-add-custom-fields-to-gooddata-pipelines branch from e4ec7a3 to 67d1d9b Compare September 15, 2025 07:54
@janmatzek janmatzek merged commit 28ac2b2 into gooddata:master Sep 15, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants