update schema

seems legit

This seems fine, I wish we had docstrings on these classes

I don't want to block this on docstrings, but I really wish we had docstrings.

i wish we had a nice way of keeping this in sync somehow

i'd prefer we not check in the -shm and -wal files and i would like to see some docs or a long changelog entry explaining to users how to migrate, what the migration will do, and what to do if they do not want to / cannot migrate.

@schrockn it might make sense for us to do this work against a feature branch rather than master

Clarify names

blk

Up

i think there are some transpositions in the docstrings

@prha let's discuss this live on monday

Postgres

hmm, this makes me so uncomfortable

Expand test

Improve test

I don't think we do actually delete the local file manager directories after the run though -- the lifecycle stuff on local_file_manager only deals with the TempfileManager, which i think was an abortive idea about providing tempfile facilities on the context

up

blk

Fixup

py2

Fixup

Is there a user we can run this by for feedback?

We should at least include an upgrade path or descriptions of what to do to upgrade. I had to blow away my whole schedule storage (without any instructions) this week because of some change and from my perspective as a user it did nothing but eat up an afternoon. I agree that in large production deployments there are scale issues but I don't like the world where scale issues / whatever are available as excuses for us not to ship migrations.

I defer to @alangenfeld but would almost rather have a script that deleted and recreated this tag each time so that it bumped to the top: https://github.com/dagster-io/dagster/releases/tag/__placeholder

Fixup

lol uh oh

Blk

Up

I am fighting the pickle fight elsewhere

Rebase

Up

I think we should add a data migration

Rebase

I don't understand what you are proposing here. If we dynamically construct a pipeline, as we do for execute_solid, there is no way to pass it across the serialization boundary. It might be helpful to consider:

Does this cause a change in behavior?

LGTM, resigning to Sashank.

Feedback

blk

Up

Up

blk

this should be good to merge after D2388 and D2390

Nits

Blk and remove extraneous check

This seems fine to me too

are you asking why vendor cloudpickle in, or why is cloudpickle or equivalent needed here? dynamically constructed pipelines (not loaded through an ExecutionTargetHandle) can't cross the serialization barrier -- so if you construct a pipeline in a pytest test that uses a fixture or in a fixture, there's no way to pass it. our stack-walking tricks, which would work if the pipeline was constructed at the top level in a test file, are fundamentally a partial implementation of cloudpickle.

this is fine or we can go with sth like https://dagster.phacility.com/D2359

How do we feel about bundling this in with a crack at https://github.com/dagster-io/dagster/issues/1087