Page MenuHomePhabricator

use instance_for_test in test_event_callback function

Authored by schrockn on Oct 29 2020, 10:08 PM.



I was looking at this test investigating flakiness and I notice it was using the forbidden method for testing (DagsterInstance.local_temp()) I doubt this will fix the issue described in #3169 but a good excuse to replace this callsite.

Test Plan


Diff Detail

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

Event Timeline

I think it might?

The thing that jumped out from that BK log output was

DagsterInstanceMigrationRequired: Instance is out of date and must be migrated (SqliteEventLogStorage for run 925f35e0-e1be-4d95-ba4e-d811849aed61). Database is at revision None, head is c34498c29964. Please run `dagster instance migrate`.

Dunno how that would happen unless the test pulled in an old instance somehow, so using instance_for_test() (which always creates a brand new instance) could very well help

This revision is now accepted and ready to land.Oct 29 2020, 10:42 PM

ok I no longer think that was it, I'm seeing this in other tests that use instance_for_test :) there's something funky going on with the event log.

got it. I reopen-ed for tracking if you want to put failures somewhere

Sent via Superhuman ( )

We can find the real underlying issue using, the migration error is masking the true cause