Page MenuHomeElementl
Differential D5082

[Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and filesystem versioned asset store.
ClosedPublic
Actions

Authored by • cdecarolis on Nov 10 2020, 5:21 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jun 2, 7:18 AM
Unknown Object (File)
Sun, May 28, 12:23 AM
Unknown Object (File)
Wed, May 24, 4:08 PM
Unknown Object (File)
Tue, May 23, 6:03 PM
Unknown Object (File)
Sun, May 21, 3:19 PM
Unknown Object (File)
Sun, May 14, 4:57 PM
Unknown Object (File)
Thu, May 11, 5:48 AM
Unknown Object (File)
Apr 22 2023, 2:37 AM
View All Files
Subscribers
None

Details

Reviewers
• sandyryza
• yuhan
Commits
R1:00956a6b4aec: [Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and…
Summary

Added an abstract class to handle storing versioned assets. This way the core API doesn't change, and doesn't require much plumbing on behalf of the user since the API will find the version itself.

Test Plan

Added new unit tests to AssetStore

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

• cdecarolis created this revision.Nov 10 2020, 5:21 PM
• cdecarolis added a parent revision: D5081: [Asset Store Versioning 1 of 3] Place computed step output versions on the execution plan..Nov 10 2020, 5:31 PM
Harbormaster completed remote builds in B20947: Diff 25409.Nov 10 2020, 5:40 PM
• cdecarolis requested review of this revision.Nov 10 2020, 5:40 PM
• cdecarolis added a child revision: D5083: [Asset Store Versioning 3 of 3] Enable use of asset store to decide which steps to rerun during memoized execution, and enable retrieval of versioned assets..Nov 10 2020, 5:50 PM
• cdecarolis updated this revision to Diff 25596.Nov 12 2020, 10:41 PM

up

Harbormaster failed remote builds in B21105: Diff 25596!Nov 12 2020, 11:00 PM
• cdecarolis updated this revision to Diff 25658.Nov 13 2020, 8:02 PM

Up

Harbormaster completed remote builds in B21151: Diff 25658.Nov 13 2020, 8:22 PM
• sandyryza accepted this revision.Nov 16 2020, 7:23 PM
• sandyryza added inline comments.
python_modules/dagster/dagster/core/storage/asset_store.py
364

Maybe remove this line?

It would be helpful to include a line explaining that the version will be part of the addressing scheme.

This revision is now accepted and ready to land.Nov 16 2020, 7:23 PM
• cdecarolis updated this revision to Diff 25856.Nov 18 2020, 12:53 AM

fixed messaging on filesystem_versioned_asset_store

Harbormaster failed remote builds in B21310: Diff 25856!Nov 18 2020, 1:08 AM
• cdecarolis updated this revision to Diff 25860.Nov 18 2020, 1:27 AM

Migrated to using asset store context

Harbormaster completed remote builds in B21313: Diff 25860.Nov 18 2020, 1:42 AM
• cdecarolis updated this revision to Diff 25861.Nov 18 2020, 1:46 AM

Fixed set_asset call

Harbormaster completed remote builds in B21314: Diff 25861.Nov 18 2020, 2:00 AM
• sandyryza added inline comments.Nov 18 2020, 4:59 PM
python_modules/dagster/dagster/core/storage/asset_store.py
354

We should call this versioned_fs_asset_store for consistency

• cdecarolis updated this revision to Diff 26020.Nov 19 2020, 8:57 PM

Removed VersionedAssetStore abstract class, and instead added a has_asset method to the base assetstore class.

Harbormaster completed remote builds in B21444: Diff 26020.Nov 19 2020, 9:12 PM
• yuhan added inline comments.Nov 20 2020, 1:16 AM
python_modules/dagster/dagster/core/storage/asset_store.py
66

nit?

249

maybe this can be moved to _get_path? your call.

286

should we pass context instead?

• cdecarolis updated this revision to Diff 26091.Nov 20 2020, 8:00 PM

Fixed nits. Re-added VersionedAssetStore abstract class so that users are not forced to implement has_asset method

Harbormaster completed remote builds in B21502: Diff 26091.Nov 20 2020, 8:15 PM
• yuhan accepted this revision.Nov 20 2020, 10:33 PM
• cdecarolis updated this revision to Diff 26545.Nov 30 2020, 4:28 PM

Up

Harbormaster completed remote builds in B21860: Diff 26545.Nov 30 2020, 4:45 PM
• cdecarolis updated this revision to Diff 26681.Dec 1 2020, 9:14 PM

Up

Harbormaster failed remote builds in B21973: Diff 26681!Dec 1 2020, 9:34 PM
• cdecarolis updated this revision to Diff 26694.Dec 1 2020, 9:44 PM

up

Harbormaster completed remote builds in B21986: Diff 26694.Dec 1 2020, 10:02 PM
• cdecarolis updated this revision to Diff 26798.Dec 2 2020, 4:18 PM

Up

Harbormaster completed remote builds in B22073: Diff 26798.Dec 2 2020, 4:38 PM
Closed by commit R1:00956a6b4aec: [Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and… (authored by dpeng817 <cdguitar817@gmail.com>, committed by Chris DeCarolis <cdguitar817@gmail.com>). · Explain WhyDec 2 2020, 4:40 PM
This revision was automatically updated to reflect the committed changes.
Chris DeCarolis <cdguitar817@gmail.com> added a commit: R1:00956a6b4aec: [Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and….

Revision Contents
Changeset List

PathSize
python_modules/
dagster/
dagster/
core/
execution/
context/
13 lines
plan/
7 lines
storage/
80 lines
dagster_tests/
core_tests/
storage_tests/
40 lines

Diff 26808

View Options

python_modules/dagster/dagster/core/execution/context/system.py

Loading...
View Options

python_modules/dagster/dagster/core/execution/plan/plan.py

Loading...
View Options

python_modules/dagster/dagster/core/storage/asset_store.py

Loading...
View Options

python_modules/dagster/dagster_tests/core_tests/storage_tests/test_asset_store.py

Loading...