Page MenuHomePhabricator

[storage] handle missing event log files
ClosedPublic

Authored by alangenfeld on Thu, Aug 22, 3:25 PM.

Details

Reviewers
schrockn
Group Reviewers
Restricted Project
Commits
R1:bb5ae923c591: [storage] handle missing event log files
Summary

and some other fixes

Test Plan
$ dagit --log

...

Could not load pipeline run from /Users/alangenfeld/data/dagster/logs/experimental/demo_repository/ccef73e9-8489-4cb9-9509-a08e9913257c.json, continuing.
  Original exception: CheckError: Invariant failed. Description: Event log file for run id ccef73e9-8489-4cb9-9509-a08e9913257c is not a file
Could not load pipeline run from /Users/alangenfeld/data/dagster/logs/experimental/demo_repository/1565370818_42a11803-d224-4100-b069-733a3a0597bd.json, continuing.
  Original exception: CheckError: Invariant failed. Description: Event log file for run id 42a11803-d224-4100-b069-733a3a0597bd is not a file
Could not load pipeline run from /Users/alangenfeld/data/dagster/logs/experimental/demo_repository/35427783-9924-4f85-b2cd-a3ef12df7b60.json, continuing.
  Original exception: CheckError: Invariant failed. Description: Event log file for run id 35427783-9924-4f85-b2cd-a3ef12df7b60 is not a file

Loading repository...
Serving on http://127.0.0.1:3000

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, Aug 22, 3:25 PM
schrockn requested changes to this revision.Thu, Aug 22, 5:43 PM
schrockn added a subscriber: schrockn.
schrockn added inline comments.
python_modules/dagster/dagster/core/storage/event_log.py
98

I'd recommend has_event_log_for_run_id or similar and then letting the caller decide what to throw because the caller has more context

This revision now requires changes to proceed.Thu, Aug 22, 5:43 PM
schrockn accepted this revision.Thu, Aug 22, 5:44 PM

Actually I'm quite wrong given that the EventLogStorage class in question has context about the underlying store.

verify_event_log seems to be the wrong name to me, since there is no verification of the actual context occuring

This revision is now accepted and ready to land.Thu, Aug 22, 5:44 PM
alangenfeld updated this revision to Diff 3947.Thu, Aug 22, 8:04 PM

verify_event_log_exists

This revision was automatically updated to reflect the committed changes.