Page MenuHomePhabricator

0.7.0 Migration Guide
ClosedPublic

Authored by schrockn on Tue, Feb 11, 9:05 PM.

Details

Summary

This starts the migration guide.

I think it is good to include error text in a migration
guide so that folks can search for it.

Test Plan

Read

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

schrockn created this revision.Tue, Feb 11, 9:05 PM
schrockn updated this revision to Diff 9533.Tue, Feb 11, 9:09 PM
schrockn edited the summary of this revision. (Show Details)
schrockn added reviewers: prha, sashank, max, alangenfeld.

upmessage

Including the folks who I believe are responsible for breaking API changes this time around. @themissinghlink @nate if you have some api changes that require migration guide here you go

alangenfeld added inline comments.Tue, Feb 11, 9:17 PM
070_MIGRATION.md
16–47

I feel like these should be structured

  • errors (like you have)
  • fix (a clear concise how-to of the fix)
  • more info (like you have on the motivation and details of the breaking change)

so people who aren't interested (or don't have the time when doing the upgrade) can quickly find the fix for each issue. Might be good to subheading these

max added inline comments.Tue, Feb 11, 9:22 PM
070_MIGRATION.md
16–47

agree

22

s/as/and

31

that can be

34

@dagster_type

36

to be usable

Won't jump on the review train, but will add: The raise_on_error change might break some pipeline tests if people are tryna catch DagsterUserCodeExceptions. They will need to reformat their tests to catch the original error instead of reaching in and grabbing the inner exception. Also depending on how they write their type_check functions (if they have custom type checks), they might get new errors. This is summarized in this revision: https://dagster.phacility.com/D1943

schrockn added inline comments.Tue, Feb 11, 9:40 PM
070_MIGRATION.md
16–47

Excellent suggestion

schrockn updated this revision to Diff 9540.Tue, Feb 11, 9:46 PM

feedback

sashank added inline comments.Tue, Feb 11, 11:11 PM
070_MIGRATION.md
2

Should this file be called 0.7.0_MIGRATION.md? We'll probably have a bunch of these where it'll be eventually ambiguous which version the non dot separated numbers refer to.

schrockn added inline comments.Tue, Feb 11, 11:12 PM
070_MIGRATION.md
2

I didn't like it either but couldn't think of anything better. Do you have any suggestions?

sashank added inline comments.Wed, Feb 12, 12:49 AM
070_MIGRATION.md
2

0.7.0_MIGRATION.md with the dots?

prha accepted this revision.Wed, Feb 12, 1:10 AM

My vote is to move to 0.7.0_MIGRATION.md, but we should get this in so others can add to it.

This revision is now accepted and ready to land.Wed, Feb 12, 1:10 AM
This revision was automatically updated to reflect the committed changes.