-
Notifications
You must be signed in to change notification settings - Fork 81
[Server][Bug] Pagination of List requests #55
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
[Server][Bug] Pagination of List requests #55
Conversation
|
Hi @chr-hertel,I have opened a PR to address the issue related to pagination. |
|
Thanks @bigdevlarry for jumping on that 👍 I think this should wait for #46 as well - you could basically use that as a base branch already |
|
#46 got merged, needs a rebase |
|
@chr-hertel this is back up for review again |
| /** | ||
| * @implements \ArrayAccess<int|string, mixed> | ||
| */ | ||
| final class Page implements \Countable, \ArrayAccess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think about extending ArrayObject instead? would maybe ease some stuff
chr-hertel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was testing this with adding more tools to example no1 and added the pagination limit on the builder - this still needs to be taken into account here - or do we better split into a follow-up PR?
anyhow, this needs a rebase after #49 and #46
Thanks already @bigdevlarry!
|
Oh, and can you please make sure to squash this into a single, signed commit please |
|
@chr-hertel I think we should split it into another PR once this one gets merged. |
10 - Added the list resources handler with test class 10 - Update the ListPromptsHandler to use annotations 10 - Add the list resource handler, and it's test as well 10 - Added the registry logic 10 - Added the list tools handler logic 10 - Fix all php-cs-fixer related issues 10 - Resolved phpstan issues for ListPromptsHandlerTest 10 - Resolved phpstan issues for ListToolsHandlerTest 10 - Resolved phpstan issues for ListResourcesHandlerTest 10 - Resolved phpstan issues in registry class 10 - Resolved phpstan issues for the handler class 10 - Resolved php cs fixer issues 10 - Resolved all phpstan issues by generating the baseline again 10 - Remove the two loop operation and use foreach for optimization 10 - Removed unused code and resolved namespace 10 - Add signatures to interface and registry 10 - Add updated phpstan rule set 10 - Arranged according to property type 10 - Introduce reference page DTO, fix all failing test 10 - Resolve cs-fixer issues 10 - Resolve phpstan issues 10 - Resolve php cs fixer issues 10 - Created a reference directory for the Page DTO 10 - Remove manual loop for clean code readability
3e63cc8 to
49e2834
Compare
chr-hertel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bigdevlarry - I squashed and rebased - let's get this merged!
Implements MCP pagination for tools/list, prompts/list, and resources/list endpoints.
Motivation and Context
The change was raised as an issue to enable the list request to have pagination support.
How Has This Been Tested?
Yes, this has been tested.
Breaking Changes
No, users don't need to make any updates
Types of changes
Checklist
Additional context