Page MenuHomePhabricator

[storage] live updating run storage
ClosedPublic

Authored by alangenfeld on Aug 21 2019, 9:10 PM.

Details

Reviewers
max
schrockn
Group Reviewers
Restricted Project
Commits
R1:09932e2d5b74: [storage] live updating run storage
Summary

origin D789

This adds the ability for the filesystem run storage to pick up runs invoked by other processes so they can be displayed in dagit.

This does not solve for live updating event logs from other processes runs - I could hack that on but I feel that is better handled after more systemic refactors.

Test Plan

stack on top of D858
run dagit --log in one window
run dagster-graphql --log in another

see live updating runs history

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

max created this revision.Aug 21 2019, 9:10 PM
max updated this revision to Diff 3909.Aug 21 2019, 9:25 PM

Rebase

max updated this revision to Diff 3911.Aug 21 2019, 9:33 PM

Rebase

alangenfeld added inline comments.Aug 21 2019, 9:40 PM
python_modules/dagit/dagit/cli.py
69

log wipe sounds odd to me

--clear-logs
--delete-all-logs

also this should really be its own diff

python_modules/dagster/dagster/core/events/log.py
26–51 ↗(On Diff #3911)

I still feel like we shouldn't need this if we are managing everything correctly - relying on this feels like a sign that we are doing something wrong architecturally

python_modules/dagster/dagster/core/storage/runs.py
125–126

rename should match that in lower diff

alangenfeld requested changes to this revision.EditedAug 21 2019, 9:50 PM

I will commandeer this

This revision now requires changes to proceed.Aug 21 2019, 9:50 PM
alangenfeld commandeered this revision.Aug 21 2019, 10:43 PM
alangenfeld edited reviewers, added: max; removed: alangenfeld.
alangenfeld edited the test plan for this revision. (Show Details)Aug 23 2019, 5:05 PM
alangenfeld updated this revision to Diff 3973.Aug 23 2019, 5:19 PM
alangenfeld edited the test plan for this revision. (Show Details)

watchdog req

alangenfeld updated this revision to Diff 3975.Aug 23 2019, 5:49 PM

only watch when running dagit

alangenfeld retitled this revision from Add naive filesystem log watchers to [storage] live updating run storage.Aug 23 2019, 5:57 PM
alangenfeld edited the summary of this revision. (Show Details)
alangenfeld updated this revision to Diff 3978.Aug 23 2019, 6:02 PM
alangenfeld edited the summary of this revision. (Show Details)

fix base take 2

natekupp added inline comments.
python_modules/dagster/dagster/core/storage/runs.py
254

Can we remove the ref to run_storage from PipelineRun, make it a namedtuple, and then add a from_json() method on that object and use it here? Would be nice to clean this up for the future when PipelineRun could be stored in a variety of serialization formats, SQL, Thrift, on floppy disks, etc.

alangenfeld added inline comments.Aug 23 2019, 6:37 PM
python_modules/dagster/dagster/core/storage/runs.py
254

aw yis

alangenfeld updated this revision to Diff 3984.Aug 23 2019, 7:27 PM

rm mistake

alangenfeld updated this revision to Diff 3985.Aug 23 2019, 7:40 PM

pull event_log_exists - wasnt working as it intended

schrockn accepted this revision.Aug 25 2019, 1:32 AM
schrockn added a subscriber: schrockn.
schrockn added inline comments.
python_modules/dagster/dagster/core/storage/pipeline_run.py
143

maybe do some argument verificaiton

This revision is now accepted and ready to land.Aug 25 2019, 1:32 AM
This revision was automatically updated to reflect the committed changes.