This resolves https://github.com/dagster-io/dagster/issues/1798 as well as performance issues stemming from calls to the default implementation of get_stats_for_run.
ya lets do the back compat dance - doesnt seem to be a lot of value to be reaped from breaking changes at this juncture
I think it might be worth shuffling classes around since I think this or related assumptions could easily be missed
SQLEventLogStorage -> SingleTableEventLogStorage -> PostgresEventLogStorage
SQLEventLogStorage -> SqliteEventLogStorage
|56 ↗||(On Diff #6734)|
worth considering removing WatchableEventLogStorage and migrating the functions on the base class and implement the few missing functions on the in-memory store
I think all our other errors are Dagster prefixed - worth staying consistent. We could get away with a single DagsterInstanceMigrationRequired error for all these types of issues
is this actually useful user facing information?
move SqlEventLogStorage to own file?
this broad except / raise_upgrade_error setup worries me a bit. If one of the lines before raise_upgrade_error = True has a typo/bad arg/whatever we will silently eat the error forever right?
should we change this test or add a different case that copies over to a temp dir, runs the migration, and ensures it works