Page MenuHomePhabricator

Expose run filtering via GraphQL and implement pagination in Dagit
ClosedPublic

Authored by bengotow on Tue, Oct 1, 11:26 PM.

Details

Summary

This diff implements server-side run filtering and client-side pagination through runs in Dagit. There are two new GraphQL endpoints that map on to the run storage interface:

pipelineRunsOrError replaces pipelineRuns. Params are cursor, limit, and a selector with one of the following:

type PipelineRunsFilter {
  runId: String
  pipeline: String
  tagKey: String
  tagValue: String
  status: PipelineRunStatus
}

Example usage:

query {
  pipelineRunsOrError(
    limit: 1,
    cursor: "9e51122f-9c6f-47c0-b92f-4bd156f4f96b",
    filter: {
      pipeline: "log_spew"
    }
  ) {
    ... on PipelineRuns {
      results {
        pipeline {
          name
        }
        runId
      }
    }
    ... on InvalidPipelineRunsFilterError {
      message
    }
  }
}

I also added pipelineRunTags, which returns all of the key-value tags currently in use and implemented it in run_storage. This allows for search autocompletion based on tags.

query {
  pipelineRunTags {
    key
    values
  }
}

The UI has been made a bit more minimal - the filter box looks the same but only allows you to put in one filter, and the solid subset option is gone. Status was merged in since you can only filter by status if you do not choose another filter, and I removed the sort box. I also removed the "most recent run" box because it was really just a normal row pulled out of the table. (Originally it was going to show extra info.)

Pagination works by requesting (displayed + 1) rows. If we want to show 50 but find that there are 51, we know there's a "next page". A next page button appears at the bottom and clicking it advances the cursor. Because we use runIds as cursor and they are non-sequential there's no way to know what the cursor for "N-50" would be, so the cursor is stored in the browser URL bar and your browser back button will take you to the previous page. The filter / search is synced to the URL bar as well so you can share links to specific filters.

I also made the UI show the tags on the runs in the Execution Params columns. Boring styling for now.
(EDIT: This seems to not work - you can request pipelineRuns { tags { key value }} via GraphQL but the tags are never returned.

Test Plan

Run tests

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

bengotow created this revision.Tue, Oct 1, 11:26 PM
prha added a subscriber: prha.Tue, Oct 1, 11:48 PM

I like the idea of having composable filters that eventually get handled by a single, merged function.

python_modules/dagster-graphql/dagster_graphql/schema/roots.py
82

maybe rename to selector?

529–530

What do you think about making cursor and limit optional args on the pipelineRunsOrError field?

They seem logically different than all the other filters, and we could then have PipelineRunsSelector be the exclusive or of run_id, pipeline, tag_key/tag_value, status, etc.

bengotow added inline comments.
python_modules/dagster-graphql/dagster_graphql/schema/roots.py
82

Hmm it looks like we have pipelineOrError(params: ) and pipeline(params: ) that take pipeline selectors so I modeled this one after those - @schrockn do you know what would be best here? I wonder if all of them should be selector actually.

529–530

Ahh that's a good idea - will move these up!

schrockn added inline comments.Wed, Oct 2, 4:31 AM
python_modules/dagster-graphql/dagster_graphql/schema/roots.py
82

we already have the notion of "Execution Selection" else where which specifies a subset of execution (subset of solids). I think all that nomenclature probably has to be rethought. I would just name as pipelineRunsSelector for now to disambiguate

529–530

👍🏻

alangenfeld requested changes to this revision.Wed, Oct 2, 2:54 PM
This revision now requires changes to proceed.Wed, Oct 2, 2:54 PM
bengotow updated this revision to Diff 5454.Wed, Oct 2, 10:40 PM
  • Client-side pagination and server-side search, addition of pipelineRunTags root query
bengotow retitled this revision from Expose run filtering via GraphQL and implement pagination in Dagit [wip] to Expose run filtering via GraphQL and implement pagination in Dagit.Wed, Oct 2, 10:48 PM
bengotow edited the summary of this revision. (Show Details)
bengotow edited the test plan for this revision. (Show Details)
bengotow edited the summary of this revision. (Show Details)
prha added a comment.Wed, Oct 2, 11:59 PM

The paging feels very weird without in-frame navigation (e.g. back button).

Could we hack something together if we sent back some summary stats from the graphql endpoint? e.g. (send back firstRunId, lastRunId, cursorIndex, count for every runs query?)

js_modules/dagit/src/TokenizingField.tsx
176

what do you think about the tab key also selecting (if and only if there's an active selection)?

js_modules/dagit/src/runs/RunsRoot.tsx
31

lol

python_modules/dagster-graphql/dagster_graphql/schema/roots.py
82

are we going to rename param?

The paging feels very weird without in-frame navigation (e.g. back button).
Could we hack something together if we sent back some summary stats from the graphql endpoint? e.g. (send back firstRunId, lastRunId, cursorIndex, count for every runs query?)

Couldn't we just keep a stack of cursors used for next and use that to power an in frame back button?

js_modules/dagit/src/runs/RunsRoot.tsx
31

first diff after we cut the release im upping to the typescript beta

js_modules/dagit/src/schema.graphql
818

I recommend renaming cursor to after and limit to first, leaves us option space for backwards pagination args (before/ last) and following the relay pagination model should align with client infra more readily

this is not blocking, we can fix this later

alangenfeld accepted this revision.Thu, Oct 3, 5:00 PM

accept in expectation of incoming changes

This revision is now accepted and ready to land.Thu, Oct 3, 5:00 PM
bengotow added inline comments.Thu, Oct 3, 5:42 PM
js_modules/dagit/src/TokenizingField.tsx
176

Ahh Nick mentioned this too - apparently I'm a weirdo that doesn't use tab :-p Will get this fixed.

js_modules/dagit/src/runs/RunsRoot.tsx
31

sounds good! 🙏

python_modules/dagster-graphql/dagster_graphql/schema/roots.py
82

Renamed to filters: of type PipelineRunsFilter after discussion w/ nick + alex

bengotow updated this revision to Diff 5499.Thu, Oct 3, 5:50 PM
  • Add cursor stack for back-pagination, rename selector to PipelineRunsFilter
bengotow edited the summary of this revision. (Show Details)Thu, Oct 3, 5:51 PM