Page MenuHomePhabricator

cdecarolis (Christopher DeCarolis)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

cdecarolis added inline comments to D6797: Launch API.
Thu, Mar 4, 11:04 PM
cdecarolis added a comment to D6797: Launch API.

One thing that I'm concerned about is communicating the failure modes of the resolution mechanism that we're doing here. It's a bit of magic, and there's a lot of ways each can fail.

Thu, Mar 4, 10:04 PM
cdecarolis requested review of D6797: Launch API.
Thu, Mar 4, 10:02 PM
cdecarolis updated the diff for D6787: [4/n init_resources] init_resources context manager.

Up

Thu, Mar 4, 9:55 PM
cdecarolis updated the diff for D6787: [4/n init_resources] init_resources context manager.

Removed comment, removed recorder

Thu, Mar 4, 9:50 PM
cdecarolis updated the diff for D6787: [4/n init_resources] init_resources context manager.

Added docstring, init_resources -> build_resources

Thu, Mar 4, 5:10 PM
cdecarolis requested review of D6787: [4/n init_resources] init_resources context manager.
Thu, Mar 4, 4:22 PM
cdecarolis requested review of D6785: [3/n init_resources] Resource initialization refactor.
Thu, Mar 4, 4:15 PM
cdecarolis updated the diff for D6741: [4/n init_resources] init_resources context manager.

up

Thu, Mar 4, 12:47 AM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Up

Thu, Mar 4, 12:42 AM

Wed, Mar 3

cdecarolis updated the diff for D6741: [4/n init_resources] init_resources context manager.

Addressing comments + propogating changes

Wed, Mar 3, 10:41 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Don't provide default values for resource initialization

Wed, Mar 3, 10:32 PM
cdecarolis added inline comments to D6727: Reconstructable from file pointing to workspace yaml.
Wed, Mar 3, 9:18 PM
cdecarolis added inline comments to D6513: [3/n init_resources] Resource initialization refactor.
Wed, Mar 3, 9:17 PM
cdecarolis added inline comments to D6741: [4/n init_resources] init_resources context manager.
Wed, Mar 3, 9:16 PM
cdecarolis updated the diff for D6741: [4/n init_resources] init_resources context manager.

Get rid of refactor artifact

Wed, Mar 3, 4:17 PM

Tue, Mar 2

cdecarolis added inline comments to D6741: [4/n init_resources] init_resources context manager.
Tue, Mar 2, 10:31 PM
cdecarolis requested review of D6741: [4/n init_resources] init_resources context manager.
Tue, Mar 2, 10:09 PM
cdecarolis requested review of D6727: Reconstructable from file pointing to workspace yaml.
Tue, Mar 2, 8:45 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Pull change into dagstermill

Tue, Mar 2, 7:08 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Up

Tue, Mar 2, 7:00 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Fixed arg on InitResourceContext

Tue, Mar 2, 6:25 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Up

Tue, Mar 2, 6:14 PM
cdecarolis retitled D6513: [3/n init_resources] Resource initialization refactor from [2/n init_resources] Resource initialization refactor to [3/n init_resources] Resource initialization refactor.
Tue, Mar 2, 4:00 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Up

Tue, Mar 2, 3:58 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Refactor artifacts

Tue, Mar 2, 3:53 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Up

Tue, Mar 2, 3:51 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Split up diff

Tue, Mar 2, 3:39 PM
cdecarolis requested review of D6740: [2/n init_resources] rename test_versioned_execution_plan.
Tue, Mar 2, 2:29 PM
cdecarolis requested review of D6739: [1/n init_resources] resolve config version fix.
Tue, Mar 2, 2:25 PM
cdecarolis retitled D6513: [3/n init_resources] Resource initialization refactor from resource initialization out-of-execution. to [2/n init_resources] Resource initialization refactor.
Tue, Mar 2, 1:53 PM
cdecarolis added inline comments to D6513: [3/n init_resources] Resource initialization refactor.
Tue, Mar 2, 1:40 PM
cdecarolis added inline comments to D6513: [3/n init_resources] Resource initialization refactor.
Tue, Mar 2, 1:30 PM

Mon, Mar 1

cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Fixed tests to account for new resource mechanism

Mon, Mar 1, 4:18 PM

Fri, Feb 26

cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Added ResourceConfig wrapper

Fri, Feb 26, 9:38 PM
cdecarolis added inline comments to D6513: [3/n init_resources] Resource initialization refactor.
Fri, Feb 26, 12:53 AM

Thu, Feb 25

cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Dagstermill up

Thu, Feb 25, 7:50 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Dagstermill up

Thu, Feb 25, 6:52 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Up

Thu, Feb 25, 4:32 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Up

Thu, Feb 25, 4:21 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Up

Thu, Feb 25, 4:14 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Major refactor, reuse most of core resource init path

Thu, Feb 25, 3:54 PM

Tue, Feb 23

cdecarolis requested review of D6640: solid decorator mypy.
Tue, Feb 23, 4:35 PM
cdecarolis accepted D6637: versioned_filesystem_io_manager requires base_dir.
Tue, Feb 23, 4:15 PM
cdecarolis added a comment to D6637: versioned_filesystem_io_manager requires base_dir.

Regarding resource initialization outside of execution; I do have a diff up moving in that direction (https://dagster.phacility.com/D6513). I do agree that it's probably better to default to requiring for now and let the dust settle on that, but my initial impression is that adding the instance would be kinda limiting. The failure cases on out of execution resource initialization are already kinda esoteric (ie my initialization requires run-specific information), and I don't feel like we fully understand that space yet.

Tue, Feb 23, 4:15 PM
cdecarolis retitled D6513: [3/n init_resources] Resource initialization refactor from [resource initialization 3/n] resource initialization pre-execution to resource initialization out-of-execution..
Tue, Feb 23, 4:11 PM
cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

UP

Tue, Feb 23, 3:46 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Fix run config passed in to init_resources from version resolution pathway

Tue, Feb 23, 3:41 PM
cdecarolis added inline comments to D6513: [3/n init_resources] Resource initialization refactor.
Tue, Feb 23, 3:23 PM

Mon, Feb 22

cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Got rid of InitResourceContext artifacts, accept run config instead of env config

Mon, Feb 22, 11:53 PM
cdecarolis added inline comments to D6513: [3/n init_resources] Resource initialization refactor.
Mon, Feb 22, 11:17 PM
cdecarolis added inline comments to D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.
Mon, Feb 22, 10:11 PM
cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

black

Mon, Feb 22, 6:17 PM
cdecarolis updated the diff for D6330: graph decorator mypy.

Up

Mon, Feb 22, 4:08 PM
cdecarolis abandoned D6418: resource initialization.
Mon, Feb 22, 4:06 PM
cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Up

Mon, Feb 22, 4:05 PM
cdecarolis updated the summary of D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.
Mon, Feb 22, 4:05 PM
cdecarolis abandoned D6494: [Pre-execution resource initialization 1/n] remove pipeline run from InitResourceContext, and replace with run id..
Mon, Feb 22, 4:03 PM
cdecarolis updated the diff for D6330: graph decorator mypy.

Up

Mon, Feb 22, 3:38 PM
cdecarolis abandoned D6514: [resource initialization 2/n] make pipeline_run optional on context creation data.

This is no longer needed.

Mon, Feb 22, 3:31 PM

Fri, Feb 19

cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Up

Fri, Feb 19, 9:54 PM
cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Up

Fri, Feb 19, 9:53 PM
cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Up

Fri, Feb 19, 8:11 PM
cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Added recursive output structure.

Fri, Feb 19, 8:06 PM

Thu, Feb 18

cdecarolis accepted D6393: validate names provided as solid alias.

if we communicate it well in a release do you think that's enough to warrant breaking?

Thu, Feb 18, 9:34 PM
cdecarolis accepted D6582: additonal test for default executor.
Thu, Feb 18, 9:33 PM
cdecarolis accepted D6580: [ez] improve make sanity_check.
Thu, Feb 18, 9:32 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Up

Thu, Feb 18, 5:51 PM
cdecarolis updated the diff for D6513: [3/n init_resources] Resource initialization refactor.

Up

Thu, Feb 18, 5:33 PM
cdecarolis accepted D6538: multiprocess configured example.

Maybe reconsider that one error message, but lgtm

Thu, Feb 18, 4:22 PM

Wed, Feb 17

cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Up

Wed, Feb 17, 4:07 PM
cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Up

Wed, Feb 17, 2:58 PM

Tue, Feb 16

cdecarolis added inline comments to D6513: [3/n init_resources] Resource initialization refactor.
Tue, Feb 16, 11:25 PM
cdecarolis added inline comments to D6513: [3/n init_resources] Resource initialization refactor.
Tue, Feb 16, 11:24 PM
cdecarolis added a comment to D6513: [3/n init_resources] Resource initialization refactor.

gonna iterate on this - not requiring an execution plan or run id.

Tue, Feb 16, 11:22 PM
cdecarolis added inline comments to D6513: [3/n init_resources] Resource initialization refactor.
Tue, Feb 16, 11:20 PM
cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Up

Tue, Feb 16, 11:17 PM
cdecarolis added inline comments to D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.
Tue, Feb 16, 11:16 PM
cdecarolis added inline comments to D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.
Tue, Feb 16, 11:14 PM
cdecarolis added a comment to D6494: [Pre-execution resource initialization 1/n] remove pipeline run from InitResourceContext, and replace with run id..

execution plan and a run id in order to initialize resources

should you even require run_id and execution_plan? I would expect pipeline_run to just be an optional property

Tue, Feb 16, 10:48 PM
cdecarolis requested review of D6514: [resource initialization 2/n] make pipeline_run optional on context creation data.
Tue, Feb 16, 10:48 PM
cdecarolis requested review of D6513: [3/n init_resources] Resource initialization refactor.
Tue, Feb 16, 10:48 PM
cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Up

Tue, Feb 16, 9:47 PM
cdecarolis requested review of D6494: [Pre-execution resource initialization 1/n] remove pipeline run from InitResourceContext, and replace with run id..
Tue, Feb 16, 9:36 PM
cdecarolis added a comment to D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Ah, gotcha, that makes sense. Thoughts on simplifying this to just pass around output_dict? I'm worried that if we make this too flexible, this will end up converging towards a duplicate of IOManager.

I also think this should probably be turned on by default. If we're encouraging people to model their solids as pure logical functions, then there isn't much much point in executing unless you can get at the outputs.

Tue, Feb 16, 3:22 PM

Mon, Feb 15

cdecarolis added a comment to D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

@cdecarolis do you mind adding a little context on the motivation for this? Are there particular situations where the reconstruct_context hack causes problems where this solution does not?

Mon, Feb 15, 5:00 PM
cdecarolis accepted D6466: pass resources in input_context for result.output_for_solid.
Mon, Feb 15, 5:00 PM

Fri, Feb 12

cdecarolis requested review of D6456: Fix step launcher documentation.
Fri, Feb 12, 4:35 PM

Thu, Feb 11

cdecarolis accepted D6430: [mypy] add some missing plan.py typing.
Thu, Feb 11, 9:25 PM
cdecarolis abandoned D6352: [execute_in_process improvements 2/n] method to retrieve output values from execution result..
Thu, Feb 11, 9:22 PM
cdecarolis abandoned D6379: [execute_in_process improvements 3/n] recursive execution result structure.
Thu, Feb 11, 9:22 PM
cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Up

Thu, Feb 11, 7:26 PM
cdecarolis updated the diff for D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.

Passed through callback fxn at top level instead of pure dict.

Thu, Feb 11, 7:19 PM
cdecarolis accepted D6419: [dynamic output] fix composites.

LGTM

Thu, Feb 11, 6:15 PM
cdecarolis requested review of D6428: [execute_in_process improvements] output values for in process execution without reconstructing context.
Thu, Feb 11, 6:00 PM
cdecarolis added a comment to D6400: type_based_io_manager.

Will take a closer look tomorrow, but the idea feels good to me. A good before-after would of course be the snowflake IO manager on internal.

Thu, Feb 11, 12:14 AM

Wed, Feb 10

cdecarolis published D6418: resource initialization for review.

Needs a little more juice to work for graphs, but the main idea is working for solids.

Wed, Feb 10, 11:46 PM

Tue, Feb 9

cdecarolis accepted D6390: CallableNode -> PendingNodeInvocation.

Thanks !

Tue, Feb 9, 10:58 PM
cdecarolis updated the diff for D6379: [execute_in_process improvements 3/n] recursive execution result structure.

Looped in adding a property for better error state when solid/graph does not have output value.

Tue, Feb 9, 5:56 PM
cdecarolis requested review of D6379: [execute_in_process improvements 3/n] recursive execution result structure.
Tue, Feb 9, 5:49 PM