Skip to content

dd limit and offset pagination support to GET /flows/{flow_id}/runs#464

Open
Neroschizoid wants to merge 1 commit intoNetflix:masterfrom
Neroschizoid:feature/add-runs-pagination
Open

dd limit and offset pagination support to GET /flows/{flow_id}/runs#464
Neroschizoid wants to merge 1 commit intoNetflix:masterfrom
Neroschizoid:feature/add-runs-pagination

Conversation

@Neroschizoid
Copy link
Copy Markdown

Summary

This PR adds support for limit and offset query parameters to the GET /flows/{flow_id}/runs endpoint to enable pagination of runs.

Changes

  • Extended AsyncPostgresTable.get_records to support an optional offset parameter.
  • Updated AsyncRunTablePostgres.get_all_runs to accept limit and offset.
  • Modified RunApi.get_all_runs to:
    • Parse limit and offset from query parameters
    • Validate integer inputs
    • Default negative values to 0
    • Return results ordered by ts_epoch DESC (latest runs first)

Behavior

  • limit=0 returns all runs (existing behavior preserved)
  • Negative values for limit and offset are clamped to 0
  • Non-integer values return a 400 error

Backward Compatibility

  • Existing calls without pagination parameters continue to behave as before.
  • No changes to default response structure.

Testing

Tested manually using:

  • ?limit=2
  • ?limit=2&offset=2
  • ?limit=0
  • ?limit=-5
  • ?limit=abc

Verified:

  • Correct pagination behavior
  • Stable ordering
  • Proper validation handling

@Aryan95614
Copy link
Copy Markdown

One thing to watch out with offset-based pagination here: on tables with concurrent inserts (common during foreach splits), rows shift between page requests which can cause duplicates or skipped records. Also offset degrades on deep pages since Postgres scans and discards N rows before returning results. Might be worth considering a cursor-based approach using ts_epoch or run_number instead. Also looks like invalid _limit values would hit the DB and return a raw 500 rather than a clean 400.

@Neroschizoid
Copy link
Copy Markdown
Author

One thing to watch out with offset-based pagination here: on tables with concurrent inserts (common during foreach splits), rows shift between page requests which can cause duplicates or skipped records. Also offset degrades on deep pages since Postgres scans and discards N rows before returning results. Might be worth considering a cursor-based approach using ts_epoch or run_number instead. Also looks like invalid _limit values would hit the DB and return a raw 500 rather than a clean 400.

Thanks for the recommendations, in my next commit i was looking into fixing the 500 status as for the cursor based approach, I would look into the query optimization to see how it plays out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants