Page MenuHomePhabricator

Enable compute log file download
ClosedPublic

Authored by prha on Tue, Sep 10, 5:22 AM.

Details

Reviewers
alangenfeld
Group Reviewers
Restricted Project
Commits
R1:4a4150f34aa8: Enable compute log file download
Summary

Adds a Flask endpoint to download the compute logs

Test Plan

Clicked download link, saw uncached, finished file

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.Tue, Sep 10, 5:22 AM
prha added a reviewer: Restricted Project.Wed, Sep 11, 1:23 AM
alangenfeld requested changes to this revision.Wed, Sep 11, 4:16 PM
alangenfeld added a subscriber: alangenfeld.
alangenfeld added inline comments.
js_modules/dagit/src/plan/ComputeLogModal.tsx
210–212

this should be computed server side and sent over GraphQL

This revision now requires changes to proceed.Wed, Sep 11, 4:16 PM
prha updated this revision to Diff 4625.Wed, Sep 11, 9:24 PM

change graphql to return file path

prha updated this revision to Diff 4626.Wed, Sep 11, 9:34 PM

fix tests

alangenfeld added inline comments.Thu, Sep 12, 5:33 PM
js_modules/dagit/src/plan/ComputeLogModal.tsx
44–48

do we want to keep fetching path during subscription?

210–213

[1]

js_modules/dagit/src/schema.graphql
27–39

hm I was thinking download_url would be more appropriate - we should ideally be able to handle the S3 case with 0 client changes [1]

alangenfeld requested changes to this revision.Thu, Sep 12, 5:36 PM
alangenfeld added inline comments.
js_modules/dagit/src/schema.graphql
27–39

we can keep path too to display where it is but we should also add download_url i think

This revision now requires changes to proceed.Thu, Sep 12, 5:36 PM
prha updated this revision to Diff 4670.Thu, Sep 12, 7:10 PM

calculate download url server-side

This revision is now accepted and ready to land.Fri, Sep 13, 6:00 PM
This revision was automatically updated to reflect the committed changes.