Skip to content

#4732 - Limit the amount of items for metastore schema API request - #4733

Open
stefan-korn wants to merge 1 commit into
GetDKAN:4.xfrom
stefan-korn:4732-limit-schema-items
Open

#4732 - Limit the amount of items for metastore schema API request#4733
stefan-korn wants to merge 1 commit into
GetDKAN:4.xfrom
stefan-korn:4732-limit-schema-items

Conversation

@stefan-korn

Copy link
Copy Markdown
Contributor

Fixes #4732

Describe your changes

QA Steps

  • Add manual QA steps in checklist format for a reviewer to perform. Be as specific as possible, provide examples if appropriate.

Checklist before requesting review

If any of these are left unchecked, please provide an explanation

  • I have updated or added tests to cover my code
  • I have updated or added documentation

@dafeder dafeder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I talked about this on the issue comments but:

  • I think if we introduce limit we should also have offset, so that people can paginate through results. However, I'm open to breaking that off for a second issue if you feel it's scope creep.
  • I think the name of the method is a little confusing. I'd suggest limitItems() - it's more concise, and makes it clear that we're applying a limit not just determining whether a limit should be there (which I'd expect to return a bool)
  • Can't merge without test coverage for limits

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.

API Metastore Schema items - provide limit parameter

2 participants