Page MenuHomePhabricator

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

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


leftover from D2959


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


  • 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


Diff Detail

R1 dagster
Automatic diff as part of commit; lint not applicable.
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