Skip to content

Conversation

@erezrokah
Copy link
Member

@erezrokah erezrokah commented Sep 25, 2025

Summary

Fixes cloudquery/cloudquery#14601

Still untested, opening for reference

Tested this in https://github.com/cloudquery/cloudquery-private/pull/9520 (internal link)


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Run go fmt to format your code 🖊
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

@github-actions github-actions bot added the feat label Sep 25, 2025
@erezrokah erezrokah changed the title feat: Support chunks in list/details patterns feat: Support chunks in resource resolvers Sep 25, 2025
@github-actions github-actions bot added feat and removed feat labels Sep 25, 2025
@erezrokah erezrokah force-pushed the feat/list_details_arrays branch from 729634a to 105f97e Compare November 3, 2025 15:00
@erezrokah erezrokah marked this pull request as ready for review November 4, 2025 18:13
@erezrokah erezrokah requested a review from a team as a code owner November 4, 2025 18:13
type RowResolver func(ctx context.Context, meta ClientMeta, resource *Resource) error

type RowsChunkResolver struct {
ChunkSize int
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need a way of ensuring ChunkSize is always less than 100 as that is what BatchSender uses

Copy link
Contributor

Choose a reason for hiding this comment

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

Or make BatchSender batch limit configurable

Copy link
Contributor

@bbernays bbernays left a comment

Choose a reason for hiding this comment

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

Overall I really like this, I do think that we should mark this as experimental so that we can break the function signature if necessary as we roll this out to our official plugins and make sure we are not missing any corner cases

@marianogappa
Copy link
Contributor

Looks legit; I didn't have @bbernays context so I would have approved it. Let's wait for that and LGTM.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feature: Support arrays in list/detail pattern

4 participants