Page MenuHomeElementl

cdecarolis (Christopher DeCarolis)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Aug 27 2021

cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Add instance to execution plan creation during execution iteration

Aug 27 2021, 2:15 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Fix memoization tests, scheduler

Aug 27 2021, 3:59 AM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Up

Aug 27 2021, 1:06 AM

Aug 26 2021

cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Get rid of nested DagsterInstance.get()

Aug 26 2021, 10:11 PM

Aug 24 2021

cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Remove instance ref shenanigans (except across GRPC endpoint)

Aug 24 2021, 3:50 PM
cdecarolis planned changes to D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

I think I can do this without the instance ref shenanigans

Aug 24 2021, 2:35 PM

Aug 19 2021

cdecarolis added reviewers for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors: alangenfeld, sandyryza.
Aug 19 2021, 4:42 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Fix integration tests

Aug 19 2021, 1:40 AM

Aug 18 2021

cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

More verbose bs test

Aug 18 2021, 10:47 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Add bs list to see what is going on

Aug 18 2021, 8:56 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Fix weird opt inst bug and airflow tests

Aug 18 2021, 7:11 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Make s3 io manager memoizable, add instance ref to k8s run launcher test instance

Aug 18 2021, 4:44 AM
cdecarolis retitled D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors from [Memoization 12/13] Fix bug with out-of-process memoization to [Memoization 12/13] Enable memoization on all out-of-the-box executors.
Aug 18 2021, 3:42 AM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Remove intermediate storage changes from demo pipeline. Too invasive to loop into this change

Aug 18 2021, 3:31 AM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Update get_external_execution_plan callsites to feed through instance ref, add celery-k8s memoization test, change instance to persistent (because it is persistent more or less)

Aug 18 2021, 2:16 AM
cdecarolis removed reviewers for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors: sandyryza, alangenfeld.
Aug 18 2021, 2:03 AM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Include instance ref when getting external execution plan

Aug 18 2021, 1:33 AM

Aug 17 2021

cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Fix run config for integration test

Aug 17 2021, 10:51 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Migrate over more tests to use s3 io manager

Aug 17 2021, 10:12 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Update integration tests

Aug 17 2021, 9:44 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Update integration test

Aug 17 2021, 7:50 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Update memoization test

Aug 17 2021, 7:46 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Use s3_pickle_io_manager in place of intermediate storage for k8s modes

Aug 17 2021, 6:51 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Up

Aug 17 2021, 6:23 PM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Add integration k8s memoization test, switch integration to use s3 pickle io manager

Aug 17 2021, 5:44 PM
cdecarolis added inline comments to D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.
Aug 17 2021, 7:54 AM
cdecarolis updated the test plan for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.
Aug 17 2021, 7:53 AM
cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Add tests for celery, dask, and k8s executors, carry forward step output versions on known state instead of execution plan.

Aug 17 2021, 7:46 AM

Aug 13 2021

cdecarolis added a comment to D9219: [memoization 13/13] revamp memoization docs.

cc @sandyryza since this stack is making heavy use of the stacked diffs workflow atm, I think switching to a PR might be a bit rough. i'll definitely put up screenshots though.

Aug 13 2021, 1:24 AM
cdecarolis updated the diff for D9219: [memoization 13/13] revamp memoization docs.

Address comments. Add tests for guide. Add tags to execute_in_process. Add resource_key to version strategy s.t. you can differentiate between resources if you choose.

Aug 13 2021, 1:21 AM

Aug 12 2021

cdecarolis updated the diff for D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.

Up

Aug 12 2021, 5:54 PM

Aug 11 2021

cdecarolis added inline comments to D9219: [memoization 13/13] revamp memoization docs.
Aug 11 2021, 5:43 AM
cdecarolis updated the test plan for D9219: [memoization 13/13] revamp memoization docs.
Aug 11 2021, 2:04 AM
cdecarolis retitled D9219: [memoization 13/13] revamp memoization docs from [memoization 13/13] revamp memoization tutorial to [memoization 13/13] revamp memoization docs.
Aug 11 2021, 2:04 AM
cdecarolis requested review of D9220: [Memoization 12/13] Enable memoization on all out-of-the-box executors.
Aug 11 2021, 1:25 AM
cdecarolis retitled D9217: [Memoization 10/13] Add top level exports for memoization + API reference page from [Memoization 10/12] Add top level exports for memoization + API reference page to [Memoization 10/13] Add top level exports for memoization + API reference page.
Aug 11 2021, 12:21 AM
cdecarolis retitled D9218: [Memoization 11/13] Make resource versions optional, even when resources are provided. from [Memoization 11/12] Make resource versions optional, even when resources are provided. to [Memoization 11/13] Make resource versions optional, even when resources are provided..
Aug 11 2021, 12:20 AM
cdecarolis retitled D9219: [memoization 13/13] revamp memoization docs from [memoization 12/12] revamp memoization tutorial to [memoization 13/13] revamp memoization tutorial.
Aug 11 2021, 12:20 AM

Aug 10 2021

cdecarolis requested review of D9219: [memoization 13/13] revamp memoization docs.
Aug 10 2021, 11:02 PM
cdecarolis requested review of D9218: [Memoization 11/13] Make resource versions optional, even when resources are provided..
Aug 10 2021, 9:44 PM
cdecarolis retitled D9217: [Memoization 10/13] Add top level exports for memoization + API reference page from [Memoization 10/n] Add top level exports for memoization + API reference page to [Memoization 10/12] Add top level exports for memoization + API reference page.
Aug 10 2021, 8:37 PM
cdecarolis retitled D9217: [Memoization 10/13] Add top level exports for memoization + API reference page from [Memoization 10/n] Add top level exports for memoization to [Memoization 10/n] Add top level exports for memoization + API reference page.
Aug 10 2021, 8:24 PM
cdecarolis updated the diff for D9217: [Memoization 10/13] Add top level exports for memoization + API reference page.

Add an API reference page for memoization

Aug 10 2021, 8:23 PM
cdecarolis requested review of D9216: [Memoization 9/n] Add version_strategy as an arg to pipeline definition..
Aug 10 2021, 12:28 AM
cdecarolis requested review of D9217: [Memoization 10/13] Add top level exports for memoization + API reference page.
Aug 10 2021, 12:27 AM

Aug 9 2021

cdecarolis added a comment to D9085: [memoization 8/n] make default fs io manager memoizable.

Addressed locally

Aug 9 2021, 10:34 PM

Aug 5 2021

cdecarolis updated the diff for D9085: [memoization 8/n] make default fs io manager memoizable.

up

Aug 5 2021, 12:41 AM

Aug 4 2021

cdecarolis updated the diff for D9085: [memoization 8/n] make default fs io manager memoizable.

Properly fail if attempting to use mapping keys with versioning.

Aug 4 2021, 11:08 PM
cdecarolis requested review of D9206: [memoization 7/n] Better error message when attempting to use memoization with dynamic orchestration.
Aug 4 2021, 9:51 PM
cdecarolis requested review of D9204: [memoization 6/n] Add validation to version strings for memoization.
Aug 4 2021, 4:57 PM
cdecarolis added inline comments to D9189: [crag] Ignore executor config when using execute_in_process.
Aug 4 2021, 4:44 PM
cdecarolis retitled D9085: [memoization 8/n] make default fs io manager memoizable from [memoization 6/n] make default fs io manager memoizable to [memoization 8/n] make default fs io manager memoizable.
Aug 4 2021, 3:11 AM
cdecarolis added inline comments to D9085: [memoization 8/n] make default fs io manager memoizable.
Aug 4 2021, 3:11 AM
cdecarolis updated the diff for D9085: [memoization 8/n] make default fs io manager memoizable.

Move version inclusion to output_identifier fxn on output context

Aug 4 2021, 2:27 AM

Aug 3 2021

cdecarolis updated the diff for D9060: [memoization 5/n] enable memoization with root input managers.

Up

Aug 3 2021, 11:14 PM
cdecarolis added inline comments to D9085: [memoization 8/n] make default fs io manager memoizable.
Aug 3 2021, 9:57 PM
cdecarolis updated the diff for D9060: [memoization 5/n] enable memoization with root input managers.

Update for usage with version strategy

Aug 3 2021, 5:24 PM
cdecarolis updated the summary of D9061: [memoization 4/n][rfc] memoization strategy for jobs.
Aug 3 2021, 1:39 AM
cdecarolis requested review of D9191: test execute_in_process with aliasing.
Aug 3 2021, 12:29 AM
cdecarolis requested review of D9139: [mypy] definitions/events.py.
Aug 3 2021, 12:23 AM
cdecarolis retitled D9190: [crag] Add validation step to default config buildup for to_job. from Add validation step to default config buildup for to_job. to [crag] Add validation step to default config buildup for to_job..
Aug 3 2021, 12:12 AM
cdecarolis requested review of D9190: [crag] Add validation step to default config buildup for to_job..
Aug 3 2021, 12:11 AM
cdecarolis requested review of D9189: [crag] Ignore executor config when using execute_in_process.
Aug 3 2021, 12:09 AM
cdecarolis abandoned D9045: [crag] Add fields not brought about in default config, fix bug with execute_in_process and default config, catch bad config earlier on ..
Aug 3 2021, 12:08 AM

Aug 2 2021

cdecarolis accepted D9172: [crag] remove job arg from partitioned schedule decorators.

See comment

Aug 2 2021, 9:18 PM
cdecarolis accepted D9184: split up test_decorators.

yay

Aug 2 2021, 9:16 PM
cdecarolis updated the diff for D9061: [memoization 4/n][rfc] memoization strategy for jobs.

Get rid of code example

Aug 2 2021, 9:13 PM
cdecarolis updated the diff for D9061: [memoization 4/n][rfc] memoization strategy for jobs.

Rebase + allow tag to toggle memoization off

Aug 2 2021, 7:45 PM

Jul 31 2021

cdecarolis requested review of D9175: Carry over field aliases to default config for job.
Jul 31 2021, 7:28 PM

Jul 30 2021

cdecarolis added a comment to D8963: [memoization 3/n] move core memoization logic to live on the execution plan.
In D8963#237434, @yuhan wrote:

in terms of user experience, i think it's fine to implicitly add a tag to a run - some other features work like that, e.g. we implicitly add parent_run_id and root_run_id as tags to new runs, and solid/step selection too. besides, it's also good that users can view those info in the Runs page such as:

image.png (802×2 px, 185 KB)

to clarify, i agree that tags are less robust as it doesn't have good checks and could get lost - that may be the trade-off we are making or config may be a better alternative bc it's schematized.

Jul 30 2021, 11:47 PM
cdecarolis updated the diff for D8963: [memoization 3/n] move core memoization logic to live on the execution plan.

Fix plumbing

Jul 30 2021, 8:00 PM
cdecarolis requested review of D9166: [bug] fix dynamic outputs issue with adls2 io manager.
Jul 30 2021, 7:03 PM
cdecarolis added a comment to D8963: [memoization 3/n] move core memoization logic to live on the execution plan.
In D8963#237378, @yuhan wrote:

Which brings us back to Alex's initial q: what's the right way to toggle memoization on and off?

I'd like to switch it on/off in dagit, so making it toggleable via either config or tag sounds good to me - if we have it defined on jobs, imo it'd be less efficient for development like ml training.

Jul 30 2021, 6:58 PM
cdecarolis added a comment to D8963: [memoization 3/n] move core memoization logic to live on the execution plan.

cc @alangenfeld regarding tags:

Jul 30 2021, 6:14 PM
cdecarolis updated the diff for D8963: [memoization 3/n] move core memoization logic to live on the execution plan.

Account for run tags to determine whether to use memoization

Jul 30 2021, 6:00 PM
cdecarolis updated the diff for D8963: [memoization 3/n] move core memoization logic to live on the execution plan.

Up

Jul 30 2021, 4:34 PM
cdecarolis updated the diff for D8963: [memoization 3/n] move core memoization logic to live on the execution plan.

Mypy execution plan snapshot, add a test to ensure that we don't carry around None values for step_output_versions

Jul 30 2021, 2:00 AM
cdecarolis added inline comments to D8963: [memoization 3/n] move core memoization logic to live on the execution plan.
Jul 30 2021, 1:17 AM

Jul 29 2021

cdecarolis added inline comments to D9061: [memoization 4/n][rfc] memoization strategy for jobs.
Jul 29 2021, 7:17 PM
cdecarolis updated the diff for D9061: [memoization 4/n][rfc] memoization strategy for jobs.

Reimplement using VersionStrategy instance, add code example

Jul 29 2021, 5:50 PM
cdecarolis added inline comments to D9061: [memoization 4/n][rfc] memoization strategy for jobs.
Jul 29 2021, 5:48 PM
cdecarolis added inline comments to D9061: [memoization 4/n][rfc] memoization strategy for jobs.
Jul 29 2021, 4:58 PM
cdecarolis updated the diff for D9061: [memoization 4/n][rfc] memoization strategy for jobs.

Implement VersionStrategy class

Jul 29 2021, 2:50 AM
cdecarolis updated the diff for D8963: [memoization 3/n] move core memoization logic to live on the execution plan.

Up

Jul 29 2021, 2:15 AM
cdecarolis updated the diff for D9059: [memoization 2/n] provide default base dir for versioned fs io manager.

Address comments

Jul 29 2021, 2:09 AM

Jul 28 2021

cdecarolis updated the summary of D9085: [memoization 8/n] make default fs io manager memoizable.
Jul 28 2021, 6:47 PM
cdecarolis requested review of D9130: [mypy] typing.NamedTuple for dependency.py.
Jul 28 2021, 6:04 PM
cdecarolis added a comment to D9061: [memoization 4/n][rfc] memoization strategy for jobs.

This strategy only really works for solids. A follow up diff will address the resources situation. Not sure if the best solution there is to keep the version argument on the resource decorator and make it not required to use memoization, or to add a resource_versioning_strategy argument (also not required) as well.

Hmm, I feel like I need to know the whole end state to evaluate this.

I think the f(solid) -> version pattern is reasonable, but should it be passed in just as a free function or something like a class with staticmethods that meets some interface MemoizationStrategy or something

Jul 28 2021, 4:59 PM
cdecarolis requested review of D9114: [mypy] leftover definition_config_schema.
Jul 28 2021, 1:20 AM
cdecarolis added a comment to D9074: [caprisun] Run ops after assets.

While I can def see the utility of the idea, this interface feels a bit off to me. From what I understand, the difference between nodes and logical assets isn't entirely cut and dry, and I'm wondering if people shouldn't be able to just add nodes to their list of assets, and we infer them as such / foist them into an asset?

Jul 28 2021, 12:08 AM
cdecarolis accepted D9098: add instructions for setting up DAGSTER_HOME to new-project README.

Nice

Jul 28 2021, 12:02 AM

Jul 27 2021

cdecarolis added a comment to D9059: [memoization 2/n] provide default base dir for versioned fs io manager.

cc @yuhan @alangenfeld might abandon this in favor of https://dagster.phacility.com/D9085, but the discussion is still relevant I think. I'm certainly open to having a versioned_outputs directory or something at that layer.

Jul 27 2021, 8:03 PM
cdecarolis abandoned D9088: [crag] Remove use of executor from execute_in_process..

This doesn't actually work

Jul 27 2021, 7:07 PM
cdecarolis requested review of D9088: [crag] Remove use of executor from execute_in_process..
Jul 27 2021, 7:03 PM
cdecarolis added a comment to D9045: [crag] Add fields not brought about in default config, fix bug with execute_in_process and default config, catch bad config earlier on ..

@alangenfeld upon further examination, I don't think the execute_plan solution can work. Even if we keep around the original executors, we still need to create a new pipeline/mode in order to switch out the io manager. Then, we'll get a different set of complaints regarding the fact that we're using a non-persistent IO manager with an out of proc executor. I think we're forced to either do the config replacement or the permissive thing.

Jul 27 2021, 6:59 PM
cdecarolis requested review of D9085: [memoization 8/n] make default fs io manager memoizable.
Jul 27 2021, 6:00 PM
cdecarolis updated the diff for D9061: [memoization 4/n][rfc] memoization strategy for jobs.

Fix tests

Jul 27 2021, 5:30 PM
cdecarolis added a comment to D9045: [crag] Add fields not brought about in default config, fix bug with execute_in_process and default config, catch bad config earlier on ..

cc @alangenfeld ^ bc I forgot to tag you

Jul 27 2021, 5:15 PM
cdecarolis added a comment to D9045: [crag] Add fields not brought about in default config, fix bug with execute_in_process and default config, catch bad config earlier on ..

have the "special" in process executor change make the whole execution section permissive at RunConfigSchema generation time - I think this is promising

I know this isn't a huge regression, but it still feels like a regression to me. I feel like it's a pretty core change for what is fundementally a workaround.

Jul 27 2021, 4:06 AM
cdecarolis updated the summary of D9061: [memoization 4/n][rfc] memoization strategy for jobs.
Jul 27 2021, 3:58 AM