Page MenuHomePhabricator

RFC - a way to run multiproc engine without sqlite event log
ClosedPublic

Authored by alangenfeld on Thu, May 14, 9:16 PM.

Details

Summary

Makes InMemoryEventLogStorage a ConfigurableClass so it can be used via overrides in DagsterInstance.local_temp to have a test instance that does not persist events to sqlite.

Test Plan

added test

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

alangenfeld created this revision.Thu, May 14, 9:16 PM
alangenfeld requested review of this revision.Thu, May 14, 9:28 PM
schrockn requested changes to this revision.Fri, May 15, 2:22 PM

Seems fine. My only request is better naming to indicate why both EphemeralEventLogStorage and InMemoryEventLogStorage exist.

This is also why ConfigurableClass should be a distinct layer that is more functional in nature. Quite the contortion here for what should just a function that creates something.

python_modules/dagster/dagster/core/storage/event_log/in_memory.py
127–142

to be clear the only purpose that this class exists is to deal with the ConfigurableClass stuff?

This revision now requires changes to proceed.Fri, May 15, 2:22 PM
alangenfeld added inline comments.Wed, May 20, 2:53 PM
python_modules/dagster/dagster/core/storage/event_log/in_memory.py
127–142

it was just to give it a different name - I can collapse it down

dont add new class

alangenfeld edited the summary of this revision. (Show Details)Wed, May 20, 3:33 PM
alangenfeld edited the summary of this revision. (Show Details)
max accepted this revision.Wed, May 20, 8:56 PM

hooray

schrockn accepted this revision.Wed, May 20, 10:16 PM
This revision is now accepted and ready to land.Wed, May 20, 10:16 PM

Can you add documentation somewhere that make it clear that this breaks dagit and and is for test only