Skip to content

Relax requirement for description and mode fields on a new node.#1418

Merged
agorajek merged 1 commit intomainfrom
node-desc-not-required
Jun 20, 2025
Merged

Relax requirement for description and mode fields on a new node.#1418
agorajek merged 1 commit intomainfrom
node-desc-not-required

Conversation

@agorajek
Copy link
Copy Markdown
Member

@agorajek agorajek commented Jun 20, 2025

Summary

For mode: just use published as the default value.

Test Plan

Unit tests.

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

Deployment Plan

Auto.

@agorajek agorajek requested a review from Copilot June 20, 2025 20:11
@netlify
Copy link
Copy Markdown

netlify bot commented Jun 20, 2025

Deploy Preview for thriving-cassata-78ae72 canceled.

Name Link
🔨 Latest commit 816d98a
🔍 Latest deploy log https://app.netlify.com/projects/thriving-cassata-78ae72/deploys/6855c0670df69f0008dad834

Copy link
Copy Markdown

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 makes the description field optional and defaults mode to published for new nodes, and adds tests to verify creating metrics without explicitly providing those fields.

  • Changed MutableNodeFields and CubeNodeFields so description is optional and added a default mode in MutableNodeFields.
  • Introduced tests in metrics_test.py to ensure nodes can be created without description or mode.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
datajunction-server/datajunction_server/models/node.py Made description optional in both models and set default mode in mutable fields
datajunction-server/tests/api/metrics_test.py Added tests for creating metrics without specifying description or mode
Comments suppressed due to low confidence (2)

datajunction-server/datajunction_server/models/node.py:666

  • For consistency with MutableNodeFields, consider giving mode a default value here (e.g., mode: NodeMode = NodeMode.PUBLISHED).
    mode: NodeMode

datajunction-server/tests/api/metrics_test.py:1310

  • After asserting the status code, add checks for the response body to confirm mode defaults to 'published' and that description is handled (e.g., assert response.json()["mode"] == "published").
    assert response.status_code == 201

@agorajek agorajek changed the title Do not require description field on a new node. Relax requirement for description and mode fields on a new node. Jun 20, 2025
@agorajek agorajek merged commit e2563f4 into main Jun 20, 2025
17 checks passed
@agorajek agorajek deleted the node-desc-not-required branch June 20, 2025 21:20
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.

3 participants