Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: flowctl catalog does need pagination after all #1626

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

jshearer
Copy link
Contributor

@jshearer jshearer commented Sep 13, 2024

Description:
I was having a weird issue where flowctl catalog pull-specs --captures=true --collections=true wasn't returning any captures, but flowctl catalog pull-specs --captures=true --collections=false was, and realized that the first command was returning 1001 specs, which is suspiciously close to PostgREST's max row limit. Turns out that even though we're paginating the inputs, we still need to paginate the queries themselves.

Existing flowctl

$ flowctl catalog pull-specs --captures=true --collections=true --prefix <...>
Wrote 1001 specifications under file:///Users/js/Documents/estuary/automatic-backfills/test/flow.yaml.

Updated flowctl

$ /usr/local/bin/flowctl catalog pull-specs --captures=true --collections=true --prefix <...>
Wrote 1732 specifications under file:///Users/js/Documents/estuary/automatic-backfills/test/flow.yaml.

This change is Reviewable

@jshearer jshearer added the change:unplanned This change is unplanned, useful for things like docs label Sep 13, 2024
@jshearer jshearer force-pushed the jshearer/flowctl_catalog_pagination branch from 4629768 to ea0c5ae Compare September 13, 2024 15:57
Copy link
Member

@jgraettinger jgraettinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Was having a weird issue where `flowctl catalog pull-specs --captures=true --collections=true` wasn't returning any captures, but `flowctl catalog pull-specs --captures=true --collections=false` was, and realized that the first command was returning exactly 1001 specs. Turns out that even though we're paginating the inputs, we still need to paginate the queries themselves.
@jshearer jshearer force-pushed the jshearer/flowctl_catalog_pagination branch from ea0c5ae to 52e9d99 Compare September 13, 2024 17:22
@jshearer
Copy link
Contributor Author

Updated to remove incorrect comment:

// No need for pagination because we're paginating the inputs.

@jshearer jshearer merged commit 5452270 into master Sep 13, 2024
3 checks passed
@jgraettinger jgraettinger deleted the jshearer/flowctl_catalog_pagination branch September 13, 2024 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:unplanned This change is unplanned, useful for things like docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants