Page MenuHomePhabricator
Differential D1221

add client-side compute log size cap
ClosedPublic
Actions

Authored by prha on Oct 4 2019, 9:44 PM.

Details

Reviewers
alangenfeld
Group Reviewers
Restricted Project
Commits
R1:c51387a521de: add client-side compute log size cap
Summary

Depends on D1219

Test Plan

Set client-side cap low, saw subscription connection close, warning sign

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.Oct 4 2019, 9:44 PM
Harbormaster failed remote builds in B4515: Diff 5630!Oct 4 2019, 9:50 PM
prha updated this revision to Diff 5638.Oct 4 2019, 10:12 PM

tests

prha edited the summary of this revision. (Show Details)Oct 4 2019, 10:15 PM
prha added a parent revision: D1219: Change compute logs graphql endpoint to query stdout/stderr separately.
prha mentioned this in D1219: Change compute logs graphql endpoint to query stdout/stderr separately.
prha added a reviewer: Restricted Project.
Harbormaster completed remote builds in B4522: Diff 5638.Oct 4 2019, 10:18 PM
nate added a subscriber: nate.Oct 4 2019, 10:21 PM

should we continue to stream logs but only retain/show the last X MB, similar to Buildkite?

sashank added a subscriber: sashank.Oct 4 2019, 10:22 PM
sashank added inline comments.
js_modules/dagit/src/plan/ComputeLogContent.tsx
79–83

Just a very minor suggestion, but maybe change this to a link that downloads the log directly? So the user doesn't have to read this and then go find the download button everytime, and instead the action is inline with the call to action.

prha updated this revision to Diff 5642.Oct 5 2019, 12:46 AM

update based on comments

Harbormaster completed remote builds in B4524: Diff 5642.Oct 5 2019, 12:54 AM
alangenfeld accepted this revision.Oct 7 2019, 2:56 PM
This revision is now accepted and ready to land.Oct 7 2019, 2:56 PM
prha mentioned this in R1:a9bae2261184: Change compute logs graphql endpoint to query stdout/stderr separately.Oct 8 2019, 12:10 AM
Closed by commit R1:c51387a521de: add client-side compute log size cap (authored by prha). · Explain WhyOct 8 2019, 12:15 AM
This revision was automatically updated to reflect the committed changes.
prha added a commit: R1:c51387a521de: add client-side compute log size cap.

Revision Contents
Changeset List

Diff 5710

View Options

js_modules/dagit/src/__tests__/__snapshots__/App.test.tsx.snap

Loading...
View Options

js_modules/dagit/src/__tests__/__snapshots__/ExecutionPlan.test.tsx.snap

Loading...
View Options

js_modules/dagit/src/__tests__/__snapshots__/ExecutionPlanBox.test.tsx.snap

Loading...
View Options

js_modules/dagit/src/plan/ComputeLogContent.tsx

Loading...
View Options

js_modules/dagit/src/plan/ComputeLogModal.tsx

Loading...