Page MenuHomePhabricator

refactor compute log manager to pull out abstract interface
ClosedPublic

Authored by prha on Sep 16 2019, 2:10 AM.

Details

Summary

In order to support both local FS and s3 implementations, we need to extract a
common interface. This diff extracts the local FS implementation, and also yanks out
all the subscription / file watching management into a separate class.

The plan is to have the s3 implementation rely on the local FS for the subscriptions /
in-flight capture, but upload to s3 as soon as the compute is done.

Depends on D1035

Test Plan

Ran unittests

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

prha created this revision.Sep 16 2019, 2:10 AM
prha updated this revision to Diff 4762.Sep 16 2019, 3:00 AM

tests

prha updated this revision to Diff 4763.Sep 16 2019, 3:18 AM

fix lint

prha added a reviewer: Restricted Project.Sep 16 2019, 3:38 AM
alangenfeld accepted this revision.Sep 17 2019, 4:39 PM
alangenfeld added a subscriber: alangenfeld.
alangenfeld added inline comments.
python_modules/dagster/dagster/core/storage/compute_log_manager.py
34–47

if immutable data classes maybe go for namedtuple or at least private members

125

separate file maybe

This revision is now accepted and ready to land.Sep 17 2019, 4:39 PM
prha updated this revision to Diff 4850.Sep 18 2019, 8:31 PM

rebase

This revision was automatically updated to reflect the committed changes.