Conversation
✅ Deploy Preview for thriving-cassata-78ae72 canceled.
|
27c3655 to
2df2335
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates cube materialization “rebuild/refresh” behavior so workflows can be regenerated without creating a new cube version, and introduces a dedicated query-service endpoint for that refresh operation.
Changes:
- Update cube node update logic to refresh materialization workflows without bumping the cube version when no other cube definition changes are present.
- Add a
refresh_cube_materializationclient method and wire it through the HTTP query client wrapper. - Update UI wording/logic around “Rebuild (latest) Materialization” and refresh button visibility; update dependencies/lockfiles.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Dependency lockfile updates / marker simplification. |
datajunction-ui/src/app/pages/NodePage/NodeMaterializationTab.jsx |
Adjust tab selection defaults and update “rebuild” messaging + latest-materialization detection. |
datajunction-server/datajunction_server/internal/nodes.py |
Refresh materializations without version bump when refresh_materialization=true and no other cube changes. |
datajunction-server/datajunction_server/service_clients.py |
Add query-service client method to POST /cubes/{cube}/refresh-materialization. |
datajunction-server/datajunction_server/query_clients/http.py |
Delegate refresh call through the HTTP query client wrapper. |
datajunction-server/tests/service_clients_test.py |
Unit tests for the new service client refresh method. |
datajunction-server/tests/query_clients/http_query_client_test.py |
Unit test that wrapper delegates refresh call. |
datajunction-server/tests/api/cubes_test.py |
API test ensuring refresh doesn’t bump cube version and reactivates matching deactivated materializations. |
datajunction-query/pyproject.toml |
Remove psycopg[async] extra (keep pool). |
Comments suppressed due to low confidence (1)
datajunction-server/datajunction_server/service_clients.py:592
- This POST to the query service does not specify a timeout. Other query-service calls (e.g., cube materialize/backfill/deactivate) set explicit timeouts, and leaving this unbounded risks hanging requests/workers if the query service is slow or unresponsive. Add an explicit timeout consistent with similar cube endpoints.
response = self.requests_session.post(
refresh_endpoint,
headers=headers,
params=params,
json=body,
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| query_service_client.refresh_cube_materialization( | ||
| cube_name=node_revision.name, | ||
| cube_version=node_revision.version, | ||
| materializations=active_mats, | ||
| request_headers=request_headers, | ||
| ) |
There was a problem hiding this comment.
refresh_materialization can be triggered even when query_service_client is a non-HTTP implementation (e.g., configured Snowflake client). In that case refresh_cube_materialization may not exist and this will raise an AttributeError at runtime. Consider guarding the call (e.g., hasattr(query_service_client, "refresh_cube_materialization") / isinstance(HttpQueryServiceClient, ...)) and returning a clear error, or add a default refresh_cube_materialization to the query-client interface with a controlled NotImplementedError and handle it here.
| query_service_client.refresh_cube_materialization( | |
| cube_name=node_revision.name, | |
| cube_version=node_revision.version, | |
| materializations=active_mats, | |
| request_headers=request_headers, | |
| ) | |
| refresh_method = getattr( | |
| query_service_client, | |
| "refresh_cube_materialization", | |
| None, | |
| ) | |
| if not callable(refresh_method): | |
| _logger.error( | |
| "Query service client %s does not support cube materialization " | |
| "refresh for cube=%s", | |
| type(query_service_client).__name__, | |
| node_revision.name, | |
| ) | |
| raise DJInvalidInputException( | |
| "Refreshing cube materializations is not supported by the " | |
| "configured query service client.", | |
| ) | |
| try: | |
| refresh_method( | |
| cube_name=node_revision.name, | |
| cube_version=node_revision.version, | |
| materializations=active_mats, | |
| request_headers=request_headers, | |
| ) | |
| except NotImplementedError as exc: # pragma: no cover | |
| _logger.error( | |
| "Cube materialization refresh is not implemented by client %s " | |
| "for cube=%s", | |
| type(query_service_client).__name__, | |
| node_revision.name, | |
| ) | |
| raise DJInvalidInputException( | |
| "Refreshing cube materializations is not supported by the " | |
| "configured query service client.", | |
| ) from exc |
Summary
Two main changes:
Test Plan
Unit tests plus by hand locally.
make checkpassesmake testshows 100% unit test coverageDeployment Plan
Auto.