Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

batch fetching asset records for asset checks #17654

Merged

Conversation

johannkm
Copy link
Contributor

@johannkm johannkm commented Nov 2, 2023

per title

@johannkm johannkm force-pushed the johann/11-02-remove_executions_resolver_on_GrapheneAssetCheck branch from 57af54a to d87dc46 Compare November 2, 2023 20:22
@johannkm johannkm force-pushed the johann/11-02-batch_fetching_asset_records_for_asset_checks branch from 4d4d14c to d4ad6d2 Compare November 2, 2023 20:22
@@ -153,16 +154,17 @@ def get_checks_for_asset(

def _execution_targets_latest_materialization(
instance: DagsterInstance,
check_key: AssetCheckKey,
asset_record: Optional[AssetRecord],
execution: AssetCheckExecutionRecord,
resolved_status: AssetCheckExecutionResolvedStatus,
) -> bool:
# always show in progress checks
if resolved_status == AssetCheckExecutionResolvedStatus.IN_PROGRESS:
Copy link
Contributor Author

@johannkm johannkm Nov 2, 2023

Choose a reason for hiding this comment

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

we could get fancy and not fetch records for in progress checks. Too small an optimization to be worth it I thnk

@johannkm johannkm requested review from sryza and rexledesma November 2, 2023 20:25
@johannkm johannkm force-pushed the johann/11-02-remove_executions_resolver_on_GrapheneAssetCheck branch from d87dc46 to f521d0a Compare November 2, 2023 20:53
@johannkm johannkm force-pushed the johann/11-02-batch_fetching_asset_records_for_asset_checks branch from d4ad6d2 to 36d5977 Compare November 2, 2023 20:53
@johannkm johannkm force-pushed the johann/11-02-remove_executions_resolver_on_GrapheneAssetCheck branch from f521d0a to 3a984c1 Compare November 2, 2023 20:59
@johannkm johannkm force-pushed the johann/11-02-batch_fetching_asset_records_for_asset_checks branch from 36d5977 to ae518cf Compare November 2, 2023 20:59
@johannkm johannkm force-pushed the johann/11-02-remove_executions_resolver_on_GrapheneAssetCheck branch from 3a984c1 to cc0dd7f Compare November 2, 2023 22:33
@johannkm johannkm force-pushed the johann/11-02-batch_fetching_asset_records_for_asset_checks branch from ae518cf to 77b5c92 Compare November 2, 2023 22:33
Base automatically changed from johann/11-02-remove_executions_resolver_on_GrapheneAssetCheck to master November 2, 2023 22:33
@johannkm johannkm force-pushed the johann/11-02-batch_fetching_asset_records_for_asset_checks branch from 77b5c92 to dcb8304 Compare November 2, 2023 22:33
@johannkm johannkm merged commit d127842 into master Nov 2, 2023
@johannkm johannkm deleted the johann/11-02-batch_fetching_asset_records_for_asset_checks branch November 2, 2023 22:34
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