Page MenuHomePhabricator

keep AssetStoreOperationType in GRAVEYARD
ClosedPublic

Authored by yuhan on Jan 14 2021, 12:34 AM.

Details

Summary

this was my bad from D5863. prior to D5863, we had ASSET_STORE_OPERATION.op stored as AssetStoreOperationType in the event log; after that we had it stored as string. The switch caused de/ser issue like https://github.com/dagster-io/dagster/issues/3533
and now as we got rid of ASSET_STORE_OPERATION and we are converting the old logs to the new ones, it'd be safer to capture them both.

This shouldn't affect users much because ASSET_STORE_OPERATION should have only existed if the user opted in to use the asset store before our cherry-pick releases or they have been developing on master.
In https://github.com/dagster-io/dagster/issues/3533 i manually deleted the affected runs. but if there is an affected user, it'd be pretty rough experience for both investigating the root cause and fixing it.

Test Plan

before

after

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

yuhan edited the test plan for this revision. (Show Details)

up

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 14 2021, 12:57 AM
Harbormaster failed remote builds in B24349: Diff 29636!
yuhan requested review of this revision.Jan 14 2021, 1:20 AM
This revision is now accepted and ready to land.Jan 14 2021, 1:26 AM
This revision was automatically updated to reflect the committed changes.