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.

Sign up for GitHub

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

Jump to bottom

Make compute asset/check subset functions async #25264

Merged
merged 6 commits into from
Oct 22, 2024
Merged

Make compute asset/check subset functions async #25264

merged 6 commits into from
Oct 22, 2024

Conversation

briantu
Copy link
Contributor

@briantu briantu commented Oct 14, 2024 •
edited
Loading

Summary & Motivation

We now want to make the compute asset and asset check subset functions on AssetGraphView async so we can use AssetStatusCacheValue.gen() instead of AssetStatusCacheValue.blocking_get(). Same for AssetCheckExecutionRecord.

How I Tested These Changes

Existing tests should pass

This was referenced Oct 14, 2024
Merged
Merged
@briantu Graphite App
Copy link
Contributor Author

briantu commented Oct 14, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @briantu and the rest of your teammates on Graphite Graphite

@briantu briantu mentioned this pull request Oct 14, 2024
Merged
@briantu briantu requested a review from OwenKephart October 14, 2024 20:25
@briantu briantu marked this pull request as ready for review October 14, 2024 20:25
OwenKephart
OwenKephart approved these changes Oct 14, 2024
View reviewed changes
Copy link
Contributor

@OwenKephart OwenKephart left a comment

Choose a reason for hiding this comment

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

Nice! Same caveat on not merging until we get to a point in the stack where we can prove performance gains but the code here looks good

@OwenKephart OwenKephart changed the base branch from briantu/make-asset-status-cache-value-loadable to master October 15, 2024 00:08
@OwenKephart OwenKephart mentioned this pull request Oct 15, 2024
Closed
@briantu briantu changed the base branch from master to briantu/make-asset-status-cache-value-loadable October 15, 2024 00:18
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from 2c11e72 to 99479fd Compare October 15, 2024 22:24
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 3bf9beb to 4b561cd Compare October 15, 2024 22:30
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from 99479fd to 2257294 Compare October 15, 2024 22:42
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 4b561cd to b0136c3 Compare October 15, 2024 22:43
@briantu briantu changed the title Make compute asset subset functions async Make compute asset/check subset functions async Oct 15, 2024
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from 2257294 to ea6002b Compare October 17, 2024 19:01
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 9329171 to 6ab40ba Compare October 17, 2024 19:01
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from ea6002b to 41e4571 Compare October 17, 2024 20:36
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 1fb4fec to 3d1e462 Compare October 17, 2024 20:36
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from 41e4571 to e5dcce0 Compare October 18, 2024 17:59
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 63527ad to 206d74c Compare October 18, 2024 17:59
@briantu briantu mentioned this pull request Oct 18, 2024
Merged
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from e5dcce0 to f57fd3a Compare October 18, 2024 22:49
@briantu briantu mentioned this pull request Oct 18, 2024
Merged
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 206d74c to 9291fd1 Compare October 18, 2024 22:49
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from f57fd3a to f3263a7 Compare October 18, 2024 23:03
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 9291fd1 to 9be1378 Compare October 18, 2024 23:03
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from f3263a7 to 31ab1fd Compare October 21, 2024 16:41
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 9be1378 to 4a1a5a3 Compare October 21, 2024 16:41
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from 31ab1fd to 5790650 Compare October 21, 2024 19:52
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 4a1a5a3 to 0535e53 Compare October 21, 2024 19:52
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from 5ae90f1 to 62c38f6 Compare October 21, 2024 22:00
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 0535e53 to 647eb5d Compare October 21, 2024 22:00
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from 62c38f6 to 2648efa Compare October 22, 2024 00:45
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 8721e6e to 6963946 Compare October 22, 2024 00:45
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from 2648efa to d696688 Compare October 22, 2024 19:39
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 6963946 to 34ba726 Compare October 22, 2024 19:39
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from d696688 to 8360c8c Compare October 22, 2024 20:24
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from b0837ff to 6e4b0df Compare October 22, 2024 20:24
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from 8360c8c to 98cf324 Compare October 22, 2024 20:31
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 6e4b0df to 59e884b Compare October 22, 2024 20:31
@briantu briantu force-pushed the briantu/make-asset-status-cache-value-loadable branch from 98cf324 to 7b08a91 Compare October 22, 2024 21:13
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 59e884b to 63c2dc8 Compare October 22, 2024 21:13
@briantu Graphite App
Copy link
Contributor Author

briantu commented Oct 22, 2024 •
edited
Loading

Merge activity

  • Oct 22, 6:08 PM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Oct 22, 6:26 PM EDT: Graphite rebased this pull request as part of a merge.
  • Oct 22, 6:27 PM EDT: A user merged this pull request with Graphite.

@briantu briantu changed the base branch from briantu/make-asset-status-cache-value-loadable to graphite-base/25264 October 22, 2024 22:21
@briantu briantu changed the base branch from graphite-base/25264 to master October 22, 2024 22:23
@briantu briantu force-pushed the briantu/make-compute-subset-async branch from 63c2dc8 to 3a0dca2 Compare October 22, 2024 22:26
@briantu briantu merged commit a2431c3 into master Oct 22, 2024
1 check failed
@briantu briantu deleted the briantu/make-compute-subset-async branch October 22, 2024 22:27
Grzyblon pushed a commit to Grzyblon/dagster that referenced this pull request Oct 26, 2024
@briantu @Grzyblon
Make compute asset/check subset functions async (dagster-io#25264)
bc6dab3 
## Summary & Motivation
We now want to make the compute asset and asset check subset functions on `AssetGraphView` async so we can use `AssetStatusCacheValue.gen()` instead of `AssetStatusCacheValue.blocking_get()`. Same for `AssetCheckExecutionRecord`.

## How I Tested These Changes
Existing tests should pass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OwenKephart OwenKephart OwenKephart approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@briantu @OwenKephart