Page MenuHomePhabricator

[dagit] make log filter follow the step selection DSL #2472
ClosedPublic

Authored by yuhan on Fri, May 22, 12:22 AM.

Details

Summary

https://github.com/dagster-io/dagster/issues/2472
leftover from D2959

code-wise:

  • lifted 2 states up: Run now owns selectedSteps and query
  • added "query" as a token in the LogFilter

behaviors:

  • when we change the DSL query, the log will follow the change and the filter input will be something like "query:step1.compute++"
    • note: i've thought about making the filter able to multi-select steps (when the DSL query changes, we change the filter input to be "step:step1, step:step2, step:3, ..."). but when there's too many steps selected or the step names are too long, the filter input would have too many/long tags. and i feel it's clear enough to just show the query and push users to use the DSL selector more.
  • same as before, change the filter input won't change the DSL query. Here we don't show suggest for "query" token so it kind of prevent users from typing DSL in the filter input.
  • query, step, type can exist at the same time and the logical operator in between is AND

Test Plan

bk

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

yuhan created this revision.Fri, May 22, 12:22 AM
yuhan requested review of this revision.Fri, May 22, 12:34 AM
yuhan edited the summary of this revision. (Show Details)Fri, May 22, 12:37 AM
yuhan added reviewers: max, schrockn, bengotow.
max added a comment.Fri, May 22, 5:41 PM

this looks great to me, seems like a straightforward improvement. i have a slight preference for the approach you've rejected of allowing multi-selection & reflecting it in the filter query

max accepted this revision.Fri, May 22, 5:41 PM
This revision is now accepted and ready to land.Fri, May 22, 5:41 PM
yuhan added a comment.Fri, May 22, 5:45 PM

@max i can maybe make the input a multi-select dropdown so it won't get too crowded for many tags. but gonna land this now cos the iteration of this isn't a blocker for 0.8.0