Page MenuHomePhabricator

cdecarolis (Christopher DeCarolis)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 20 2020, 3:42 PM (18 w, 5 d)

Recent Activity

Wed, Nov 25

cdecarolis published D5275: adls2 asset store for review.

build completed successfully, not sure why the hook didn't publish.

Wed, Nov 25, 7:50 PM
cdecarolis updated the diff for D5274: gcs asset store.

Up

Wed, Nov 25, 4:32 PM

Tue, Nov 24

cdecarolis updated the diff for D5274: gcs asset store.

Up

Tue, Nov 24, 10:14 PM
cdecarolis updated the diff for D5252: s3 asset store.

Up

Tue, Nov 24, 9:55 PM
cdecarolis updated the diff for D5252: s3 asset store.

Up

Tue, Nov 24, 9:49 PM
cdecarolis updated the diff for D5252: s3 asset store.

Up

Tue, Nov 24, 9:29 PM
cdecarolis added inline comments to D5274: gcs asset store.
Tue, Nov 24, 9:24 PM
cdecarolis requested review of D5274: gcs asset store.
Tue, Nov 24, 9:05 PM
cdecarolis updated the diff for D5252: s3 asset store.

Addressed comments

Tue, Nov 24, 9:01 PM

Mon, Nov 23

cdecarolis requested review of D5252: s3 asset store.
Mon, Nov 23, 9:59 PM

Fri, Nov 20

cdecarolis updated the diff for 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..

Up

Fri, Nov 20, 10:29 PM
cdecarolis updated the diff for 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..

Up

Fri, Nov 20, 8:39 PM
cdecarolis added reviewers for 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.: alangenfeld, prha.
Fri, Nov 20, 8:32 PM
cdecarolis updated the diff for D5082: [Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and filesystem versioned asset store..

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

Fri, Nov 20, 8:00 PM
cdecarolis updated the diff for D5081: [Asset Store Versioning 1 of 3] Place computed step output versions on the execution plan..

Up

Fri, Nov 20, 4:50 PM
cdecarolis added inline comments to 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..
Fri, Nov 20, 3:36 PM

Thu, Nov 19

cdecarolis updated the diff for D5081: [Asset Store Versioning 1 of 3] Place computed step output versions on the execution plan..

Up

Thu, Nov 19, 9:37 PM
cdecarolis updated the diff for 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..

Rebased on top of Versioned Asset Store changes

Thu, Nov 19, 8:59 PM
cdecarolis updated the diff for D5082: [Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and filesystem versioned asset store..

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

Thu, Nov 19, 8:57 PM
cdecarolis updated the diff for D5081: [Asset Store Versioning 1 of 3] Place computed step output versions on the execution plan..

Rebased on top of AssetStoreContext changes

Thu, Nov 19, 8:54 PM
cdecarolis accepted D5190: asset-store AssetStoreContext 3/ nullable pipeline run and include execution plan.

LGTM! I think these changes are gonna enable the versioning workstreams in a big way.

Thu, Nov 19, 4:45 PM
cdecarolis added inline comments to D5190: asset-store AssetStoreContext 3/ nullable pipeline run and include execution plan.
Thu, Nov 19, 1:00 AM
cdecarolis accepted D5190: asset-store AssetStoreContext 3/ nullable pipeline run and include execution plan.

LGTM! Fwiw @sandyryza I think exposing the construct method makes sense, if at least for the testing case that we talked about before.

Thu, Nov 19, 12:59 AM
cdecarolis added inline comments to D5190: asset-store AssetStoreContext 3/ nullable pipeline run and include execution plan.
Thu, Nov 19, 12:32 AM

Wed, Nov 18

cdecarolis requested changes to D5190: asset-store AssetStoreContext 3/ nullable pipeline run and include execution plan.

to your queue

Wed, Nov 18, 10:46 PM
cdecarolis added inline comments to D5190: asset-store AssetStoreContext 3/ nullable pipeline run and include execution plan.
Wed, Nov 18, 9:25 PM
cdecarolis updated the diff for D5082: [Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and filesystem versioned asset store..

Fixed set_asset call

Wed, Nov 18, 1:46 AM
cdecarolis updated the diff for D5082: [Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and filesystem versioned asset store..

Migrated to using asset store context

Wed, Nov 18, 1:27 AM
cdecarolis updated the diff for D5082: [Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and filesystem versioned asset store..

fixed messaging on filesystem_versioned_asset_store

Wed, Nov 18, 12:53 AM

Tue, Nov 17

cdecarolis updated the diff for D5081: [Asset Store Versioning 1 of 3] Place computed step output versions on the execution plan..

Up

Tue, Nov 17, 11:19 PM
cdecarolis updated the diff for D5081: [Asset Store Versioning 1 of 3] Place computed step output versions on the execution plan..

Addressed comments. Moved version resolution logic off of the execution plan. passed mode as a param of environment config instead of mode_definition to execution plan. Got rid of outdated documentation. Assert -> check invariant.

Tue, Nov 17, 11:17 PM
cdecarolis added inline comments to D5089: asset-store introduce AssetStoreContext.
Tue, Nov 17, 8:12 PM
cdecarolis updated the diff for 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..

Up

Tue, Nov 17, 2:59 PM

Fri, Nov 13

cdecarolis updated the diff for 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..

Fixed lint issue

Fri, Nov 13, 8:49 PM
cdecarolis updated the diff for 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..

upUp

Fri, Nov 13, 8:11 PM
cdecarolis updated the diff for D5082: [Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and filesystem versioned asset store..

Up

Fri, Nov 13, 8:02 PM
cdecarolis updated the diff for D5081: [Asset Store Versioning 1 of 3] Place computed step output versions on the execution plan..

Up

Fri, Nov 13, 7:17 PM
cdecarolis updated the diff for D5081: [Asset Store Versioning 1 of 3] Place computed step output versions on the execution plan..

Use topological steps

Fri, Nov 13, 6:25 PM

Thu, Nov 12

cdecarolis added a comment to D5081: [Asset Store Versioning 1 of 3] Place computed step output versions on the execution plan..

This might work for the in-process executor, but I suspect that, for other executors, this will encounter difficulties, because the execution plans inside the steps will be constructed via a path that doesn't involve resolve_memoized_execution_plan.

More generally - ExecutionPlans are produced and consumed from a variety of locations in the codebase. If the presence of the step output versions depends on where the execution plan was created, it requires readers to trace back to where it was created in order to understand how it can be used.

I wonder if it instead makes sense to resolve step versions inside the execution plan as part of its construction.

Thu, Nov 12, 10:45 PM
cdecarolis updated the diff for D5082: [Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and filesystem versioned asset store..

up

Thu, Nov 12, 10:41 PM
cdecarolis updated the diff for D5081: [Asset Store Versioning 1 of 3] Place computed step output versions on the execution plan..

Placed computed fxn to compute step output versions on execution plan using information already provided in step builder

Thu, Nov 12, 10:18 PM

Tue, Nov 10

cdecarolis requested review of 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..
Tue, Nov 10, 6:18 PM
cdecarolis requested review of D5082: [Asset Store Versioning 2 of 3] Added VersionedAssetStore abstract class and filesystem versioned asset store..
Tue, Nov 10, 5:40 PM
cdecarolis requested review of D5081: [Asset Store Versioning 1 of 3] Place computed step output versions on the execution plan..
Tue, Nov 10, 5:36 PM

Wed, Nov 4

cdecarolis requested review of D5023: python typeddict type loader.
Wed, Nov 4, 5:16 PM

Mon, Nov 2

cdecarolis requested review of D4999: Revert "python typeddict type loader".
Mon, Nov 2, 7:45 PM
cdecarolis added a reverting change for D4952: python typeddict type loader: D4999: Revert "python typeddict type loader".
Mon, Nov 2, 7:26 PM
cdecarolis added a reverting change for R1:c3af43a5cb32: python typeddict type loader: D4999: Revert "python typeddict type loader".
Mon, Nov 2, 7:26 PM
cdecarolis updated the diff for D4952: python typeddict type loader.

Improved comments

Mon, Nov 2, 6:21 PM
cdecarolis added inline comments to D4952: python typeddict type loader.
Mon, Nov 2, 6:11 PM
cdecarolis added inline comments to D4976: add mem_asset_store, support different asset stores per solid.
Mon, Nov 2, 4:14 PM
cdecarolis updated the diff for D4952: python typeddict type loader.

black

Mon, Nov 2, 3:51 PM

Fri, Oct 30

cdecarolis updated the diff for D4952: python typeddict type loader.

Added additional failure mode tests. When non-str key provided, no type loader is set.

Fri, Oct 30, 7:34 PM

Oct 29 2020

cdecarolis added inline comments to D4952: python typeddict type loader.
Oct 29 2020, 9:23 PM
cdecarolis updated the diff for D4952: python typeddict type loader.

Added test for when inner type without loader is used

Oct 29 2020, 8:40 PM
cdecarolis requested review of D4954: Test for python set's dagstertypeloader.
Oct 29 2020, 4:36 PM
cdecarolis accepted D4940: Revert dagster-aws-pyspark.

thanks for fix!

Oct 29 2020, 4:30 PM
cdecarolis updated the summary of D4952: python typeddict type loader.
Oct 29 2020, 4:19 PM
cdecarolis requested review of D4952: python typeddict type loader.
Oct 29 2020, 4:16 PM

Oct 22 2020

cdecarolis accepted D4867: [docs] fix types overview.

Looks good!

Oct 22 2020, 8:51 PM
cdecarolis accepted D4869: [docs] fix rest of overview sections.

Thanks so much for fixing these up.

Oct 22 2020, 8:51 PM

Oct 20 2020

cdecarolis added inline comments to D4830: Yet another line number fix 😔.
Oct 20 2020, 4:58 PM
cdecarolis requested review of D4830: Yet another line number fix 😔.
Oct 20 2020, 4:20 PM

Oct 13 2020

cdecarolis added inline comments to D4713: This adds a CLI to print the stored versions of each output produced by a pipeline. It can be executed by running `dagster pipeline print_versions <options>`..
Oct 13 2020, 3:11 PM

Oct 12 2020

cdecarolis accepted D4738: have IntermediateStorage determine version addresses.
Oct 12 2020, 10:38 PM
cdecarolis updated the diff for D4755: Fixed non matching line numbers across all examples and tutorials. Fixed mismatched code snippets. Changed from line numbers to markers where possible..

docs make black

Oct 12 2020, 10:03 PM
cdecarolis updated the diff for D4751: suppressed experimental arg warning for builtin dagster type loaders..

Added issue tracking

Oct 12 2020, 7:25 PM
cdecarolis added inline comments to D4751: suppressed experimental arg warning for builtin dagster type loaders..
Oct 12 2020, 7:14 PM
cdecarolis requested review of D4751: suppressed experimental arg warning for builtin dagster type loaders..
Oct 12 2020, 7:05 PM
cdecarolis accepted D4746: Uses comment markers instead of line numbers..

Looks good. Thanks for catching!

Oct 12 2020, 4:56 PM
cdecarolis abandoned D4730: Enabled single address to point to multiple step outputs..
Oct 12 2020, 4:11 PM

Oct 9 2020

cdecarolis added a comment to D4738: have IntermediateStorage determine version addresses.

Awesome stuff - just want to make sure I understand the breaking correctly here: A call such as Output("value", address="some_address") is no longer permitted?

Oct 9 2020, 6:02 PM
cdecarolis updated the diff for D4713: This adds a CLI to print the stored versions of each output produced by a pipeline. It can be executed by running `dagster pipeline print_versions <options>`..

Changed print_versions -> list_versions. Consolidated config option. Made list_versions logic more concise.

Oct 9 2020, 4:25 PM

Oct 8 2020

cdecarolis requested review of D4730: Enabled single address to point to multiple step outputs..
Oct 8 2020, 8:13 PM
cdecarolis requested review of D4727: Enabled versioning for commonly used builtin types when loading inputs from config..
Oct 8 2020, 7:05 PM
cdecarolis updated the summary of D4713: This adds a CLI to print the stored versions of each output produced by a pipeline. It can be executed by running `dagster pipeline print_versions <options>`..
Oct 8 2020, 3:41 PM
cdecarolis updated the summary of D4713: This adds a CLI to print the stored versions of each output produced by a pipeline. It can be executed by running `dagster pipeline print_versions <options>`..
Oct 8 2020, 3:40 PM
cdecarolis requested review of D4713: This adds a CLI to print the stored versions of each output produced by a pipeline. It can be executed by running `dagster pipeline print_versions <options>`..
Oct 8 2020, 3:23 PM

Oct 7 2020

cdecarolis added a comment to D4678: factor ObjectStoreOperation creation into IntermediateStorage.

I'm not too familiar with ObjectStoreOperation's fxnality / how this ultimately ties back to the addressing stuff. Deferring to @yuhan here.

Oct 7 2020, 10:01 PM
cdecarolis updated the diff for D4659: Enabled hydration of step inputs from address when address exists. Added test boilerplate for entire memoized dev loop..

Got rid of some really questionable control flow.

Oct 7 2020, 6:54 PM
cdecarolis accepted D4701: Fix highlighting and use markers for literal includes.

I must have missed this one with my fix. Thanks so much for catching it! LGTM

Oct 7 2020, 6:21 PM
cdecarolis accepted D4696: with memoized reexecution, only copy outputs that current plan won't generate.

LGTM!

Oct 7 2020, 3:32 PM
cdecarolis updated the diff for D4659: Enabled hydration of step inputs from address when address exists. Added test boilerplate for entire memoized dev loop..

Got rid of test_build_memoized_plan, was subsumed by other tests.

Oct 7 2020, 3:22 PM
cdecarolis updated the diff for D4659: Enabled hydration of step inputs from address when address exists. Added test boilerplate for entire memoized dev loop..

Fixed lint errors

Oct 7 2020, 3:15 PM
cdecarolis updated the diff for D4659: Enabled hydration of step inputs from address when address exists. Added test boilerplate for entire memoized dev loop..

Fixed issue with step execution that was preventing hydration. Changed pipeline to actually create result files. Renamed and moved files since they no longer directly test the CLI.

Oct 7 2020, 3:08 PM

Oct 6 2020

cdecarolis updated the diff for D4659: Enabled hydration of step inputs from address when address exists. Added test boilerplate for entire memoized dev loop..

Up

Oct 6 2020, 2:35 PM

Oct 5 2020

cdecarolis updated the diff for D4659: Enabled hydration of step inputs from address when address exists. Added test boilerplate for entire memoized dev loop..

Up

Oct 5 2020, 10:48 PM
cdecarolis updated the diff for D4658: added addresses to StepInput.

up

Oct 5 2020, 10:47 PM
cdecarolis updated the diff for D4659: Enabled hydration of step inputs from address when address exists. Added test boilerplate for entire memoized dev loop..

Up

Oct 5 2020, 9:37 PM
cdecarolis updated the diff for D4658: added addresses to StepInput.

Up

Oct 5 2020, 9:37 PM
cdecarolis updated the diff for D4659: Enabled hydration of step inputs from address when address exists. Added test boilerplate for entire memoized dev loop..

Up

Oct 5 2020, 9:32 PM
cdecarolis updated the diff for D4658: added addresses to StepInput.

Up

Oct 5 2020, 9:31 PM
cdecarolis updated the diff for D4659: Enabled hydration of step inputs from address when address exists. Added test boilerplate for entire memoized dev loop..

Fixed nits. Altered partial versioning test to fit with changes

Oct 5 2020, 9:23 PM
cdecarolis updated the diff for D4658: added addresses to StepInput.

Removed implementation-specific test. Added type checking to values in address dict

Oct 5 2020, 8:50 PM
cdecarolis added inline comments to D4658: added addresses to StepInput.
Oct 5 2020, 3:32 PM
cdecarolis requested review of D4659: Enabled hydration of step inputs from address when address exists. Added test boilerplate for entire memoized dev loop..
Oct 5 2020, 3:27 PM
cdecarolis added a comment to D4658: added addresses to StepInput.

theres nothing objectionable in here but I actually think a bigger more fleshed out diff that actually uses this newly added information would be preferable.

Oct 5 2020, 3:22 PM
cdecarolis updated the test plan for D4662: fixed line numbers on various docs.
Oct 5 2020, 3:21 PM
cdecarolis accepted D4650: enable partial versioning of a pipeline.
Oct 5 2020, 3:01 PM
cdecarolis added a comment to D4650: enable partial versioning of a pipeline.

The only thing that worries me is how this might play with the part of the system that actually caches addresses for a given version. But as long as we're intentional going forward with preserving this behavior it should be fine. The fact that steps upstream of a step with a filled-in version really helps us here.

Oct 5 2020, 3:01 PM