Skip to content

[dg] Add asset checks to dg list defs#29016

Merged
OwenKephart merged 1 commit intomasterfrom
04-04-_dg_add_asset_checks_to_dg_list_defs
Apr 7, 2025
Merged

[dg] Add asset checks to dg list defs#29016
OwenKephart merged 1 commit intomasterfrom
04-04-_dg_add_asset_checks_to_dg_list_defs

Conversation

@OwenKephart
Copy link
Contributor

@OwenKephart OwenKephart commented Apr 4, 2025

Summary & Motivation

Now we can pass info about asset checks through list defs

How I Tested These Changes

Screenshot 2025-04-04 at 12.19.32 PM.png

Also added an asset check to our unit tests

Changelog

dg list defs now includes infomation about asset checks.

Copy link
Contributor Author

OwenKephart commented Apr 4, 2025

Copy link
Member

@schrockn schrockn left a comment

Choose a reason for hiding this comment

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

can you post screenshot as test plan.

also a snapshot test would be good

Copy link
Member

@schrockn schrockn left a comment

Choose a reason for hiding this comment

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

"can you post screenshot as test plan.

also a snapshot test would be good"

@OwenKephart OwenKephart force-pushed the 04-04-_dg_add_asset_checks_to_dg_list_defs branch from 1d82ad7 to 897b5fd Compare April 4, 2025 19:21
@OwenKephart OwenKephart requested a review from schrockn April 4, 2025 19:22
@OwenKephart OwenKephart force-pushed the 04-04-_dg_add_asset_checks_to_dg_list_defs branch from 897b5fd to dac0a03 Compare April 4, 2025 19:27
@OwenKephart OwenKephart force-pushed the 04-04-_dg_use_whitelist_for_serdes_for_dgdefinitionmetadata branch from 557a521 to 363a4b6 Compare April 5, 2025 00:07
@OwenKephart OwenKephart force-pushed the 04-04-_dg_add_asset_checks_to_dg_list_defs branch from dac0a03 to 8266845 Compare April 5, 2025 00:07
Comment on lines 197 to 234
│ │ │ my_schedule │ @daily │ │
│ │ └─────────────┴───────────────┘ │
│ Sensor │ ┏━━━━━━━━━━━┓ │
│ │ ┃ Name ┃ │
│ │ ┡━━━━━━━━━━━┩ │
│ │ │ my_sensor │ │
│ │ └───────────┘ │
└──────────┴───────────────────────────────────────────────────────┘
""").strip()

_EXPECTED_DEFS_JSON = textwrap.dedent("""
[
{
"type": "asset",
"key": "my_asset_1",
"group": "default",
"deps": [],
"kinds": [],
"group": "default",
"description": null,
"automation_condition": null
},
{
"type": "asset",
"key": "my_asset_2",
"group": "default",
"deps": [],
"kinds": [],
"group": "default",
"description": null,
"automation_condition": null
},
{
"type": "job",
"name": "my_job"
},
{
"type": "schedule",
"name": "my_schedule",
"cron_schedule": "@daily"
},
{
"type": "sensor",
"name": "my_sensor"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The JSON test expectations appear to be missing the type field for each object. In the updated output format, each definition should include a field indicating whether it's an asset, job, schedule, etc., but these type fields have been removed from the test expectations. The actual output likely still includes these type identifiers, which would cause the tests to fail when comparing expected vs. actual output.

Suggested change
│ │ │ my_schedule │ @daily │ │
│ │ └─────────────┴───────────────┘ │
Sensor │ ┏━━━━━━━━━━━┓ │
│ │ ┃ Name ┃ │
│ │ ┡━━━━━━━━━━━┩ │
│ │ │ my_sensor │ │
│ │ └───────────┘ │
└──────────┴───────────────────────────────────────────────────────┘
""").strip()
_EXPECTED_DEFS_JSON = textwrap.dedent("""
[
{
"type": "asset",
"key": "my_asset_1",
"group": "default",
"deps": [],
"kinds": [],
"group": "default",
"description": null,
"automation_condition": null
},
{
"type": "asset",
"key": "my_asset_2",
"group": "default",
"deps": [],
"kinds": [],
"group": "default",
"description": null,
"automation_condition": null
},
{
"type": "job",
"name": "my_job"
},
{
"type": "schedule",
"name": "my_schedule",
"cron_schedule": "@daily"
},
{
"type": "sensor",
"name": "my_sensor"
}
│ │ │ my_schedule │ @daily │ │
│ │ └─────────────┴───────────────┘ │
Sensor │ ┏━━━━━━━━━━━┓ │
│ │ ┃ Name ┃ │
│ │ ┡━━━━━━━━━━━┩ │
│ │ │ my_sensor │ │
│ │ └───────────┘ │
└──────────┴───────────────────────────────────────────────────────┘
""").strip()
_EXPECTED_DEFS_JSON = textwrap.dedent("""
[
{
"key": "my_asset_1",
"deps": [],
"kinds": [],
"group": "default",
"description": null,
"automation_condition": null,
"type": "asset"
},
{
"key": "my_asset_2",
"deps": [],
"kinds": [],
"group": "default",
"description": null,
"automation_condition": null,
"type": "asset"
},
{
"name": "my_job",
"type": "job"
},
{
"name": "my_schedule",
"cron_schedule": "@daily",
"type": "schedule"
},
{
"name": "my_sensor",
"type": "sensor"
}

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

@github-actions
Copy link

github-actions bot commented Apr 5, 2025

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-803wnv7up-elementl.vercel.app
https://04-04-dg-add-asset-checks-to-dg-list-defs.archive.dagster-docs.io

Direct link to changed pages:

@OwenKephart OwenKephart force-pushed the 04-04-_dg_add_asset_checks_to_dg_list_defs branch from 8266845 to da6e843 Compare April 7, 2025 16:10
@OwenKephart OwenKephart force-pushed the 04-04-_dg_use_whitelist_for_serdes_for_dgdefinitionmetadata branch from 363a4b6 to 857b508 Compare April 7, 2025 16:10
@OwenKephart OwenKephart force-pushed the 04-04-_dg_add_asset_checks_to_dg_list_defs branch from da6e843 to 0c980ae Compare April 7, 2025 16:31
Copy link
Contributor Author

OwenKephart commented Apr 7, 2025

Merge activity

  • Apr 7, 12:34 PM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Apr 7, 12:51 PM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 7, 12:52 PM EDT: A user merged this pull request with Graphite.

@OwenKephart OwenKephart changed the base branch from 04-04-_dg_use_whitelist_for_serdes_for_dgdefinitionmetadata to graphite-base/29016 April 7, 2025 16:48
@OwenKephart OwenKephart changed the base branch from graphite-base/29016 to master April 7, 2025 16:49
@OwenKephart OwenKephart force-pushed the 04-04-_dg_add_asset_checks_to_dg_list_defs branch from 0c980ae to 3338168 Compare April 7, 2025 16:50
@OwenKephart OwenKephart merged commit 14edab0 into master Apr 7, 2025
3 of 5 checks passed
@OwenKephart OwenKephart deleted the 04-04-_dg_add_asset_checks_to_dg_list_defs branch April 7, 2025 16:52
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