Skip to content

REST: Add support for page-size in list_namespaces, list_tables, and list_views#3377

Merged
kevinjqliu merged 2 commits into
apache:mainfrom
ebyhr:ebi/list-views-page-size
May 24, 2026
Merged

REST: Add support for page-size in list_namespaces, list_tables, and list_views#3377
kevinjqliu merged 2 commits into
apache:mainfrom
ebyhr:ebi/list-views-page-size

Conversation

@ebyhr
Copy link
Copy Markdown
Member

@ebyhr ebyhr commented May 16, 2026

Rationale for this change

Follows REST catalog spec:

Are these changes tested?

Yes.

Are there any user-facing changes?

Add support for page-size in list_views in REST catalog.

Copy link
Copy Markdown
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

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

LGTM

now that list_namespace (#3347), list_tables (#3348) and list_views (#3349) are all merged, could we use this PR to add support for page-size for all of them?

Comment thread pyiceberg/catalog/rest/__init__.py Outdated
Comment on lines +1131 to +1136
params = {}
page_size = property_as_int(self.properties, PAGE_SIZE, None)
if page_size is not None:
if page_size <= 0:
raise ValueError(f"{PAGE_SIZE} must be a positive integer")
params["pageSize"] = str(page_size)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
params = {}
page_size = property_as_int(self.properties, PAGE_SIZE, None)
if page_size is not None:
if page_size <= 0:
raise ValueError(f"{PAGE_SIZE} must be a positive integer")
params["pageSize"] = str(page_size)
page_size = property_as_int(self.properties, PAGE_SIZE, None)
if page_size is not None and page_size <= 0:
raise ValueError(f"{PAGE_SIZE} must be a positive integer")

nit

Comment on lines +1142 to +1143
if page_token:
params["pageToken"] = page_token
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if page_token:
params["pageToken"] = page_token
params: dict[str, str] = {}
if page_size is not None:
params["pageSize"] = str(page_size)
if page_token:
params["pageToken"] = page_token

nit: follows the same pattern as list_namespaces (#3347) and list_tables (#3348)

Copy link
Copy Markdown
Member Author

@ebyhr ebyhr May 24, 2026

Choose a reason for hiding this comment

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

The page size remains constant throughout the while loop, unlike the page token. That is why I set the value outside the loop.

Let me know if you still want me to commit this suggestion.

@ebyhr ebyhr force-pushed the ebi/list-views-page-size branch from 9377d82 to ceb612c Compare May 24, 2026 00:23
@ebyhr ebyhr changed the title REST: Add support for page-size in list_views REST: Add support for page-size in list_namespaces, list_tables, and list_views May 24, 2026
Copy link
Copy Markdown
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@kevinjqliu kevinjqliu merged commit 8055a60 into apache:main May 24, 2026
16 checks passed
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