Skip to content

Comments

Few changes and fixes to cube's Materialization.#1785

Open
agorajek wants to merge 1 commit intomainfrom
DJ-136
Open

Few changes and fixes to cube's Materialization.#1785
agorajek wants to merge 1 commit intomainfrom
DJ-136

Conversation

@agorajek
Copy link
Member

@agorajek agorajek commented Feb 24, 2026

Summary

Two main changes:

  1. Don't bump node version when adding/refreshing materialization.
  2. Add a new end point for materialization refreshes. Not critical, but nice to have.

Test Plan

Unit tests plus by hand locally.

  • PR has an associated issue: #
  • make check passes
  • make test shows 100% unit test coverage

Deployment Plan

Auto.

@netlify
Copy link

netlify bot commented Feb 24, 2026

Deploy Preview for thriving-cassata-78ae72 canceled.

Name Link
🔨 Latest commit b5c601e
🔍 Latest deploy log https://app.netlify.com/projects/thriving-cassata-78ae72/deploys/699d2f6e0647e00008c824ff

@agorajek agorajek force-pushed the DJ-136 branch 2 times, most recently from 27c3655 to 2df2335 Compare February 24, 2026 03:42
@agorajek agorajek changed the title Add a refresh_cube_materialization end point and adjust the UI messag… Few changes and fixes to cube's Materialization. Feb 24, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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_materialization client 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.

Comment on lines +1340 to +1345
query_service_client.refresh_cube_materialization(
cube_name=node_revision.name,
cube_version=node_revision.version,
materializations=active_mats,
request_headers=request_headers,
)
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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

Copilot uses AI. Check for mistakes.
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.

1 participant