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

Feat/paging #1572

Merged
merged 32 commits into from
Jun 4, 2022
Merged

Feat/paging #1572

merged 32 commits into from
Jun 4, 2022

Conversation

calebkiage
Copy link
Contributor

@calebkiage calebkiage commented May 17, 2022

Closes #1569

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Good first draft!
Beside the comment I left, please fix the failing tests and add an entry to the changelog.
Also thank you for setting the friendly assembly!

src/Kiota.Builder/CodeDOM/CodeMethod.cs Show resolved Hide resolved
Copy link
Member

@MIchaelMainer MIchaelMainer left a comment

Choose a reason for hiding this comment

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

Coming into this without context....

Is it correct to call this OpenApi paging as this looks like Microsoft Graph paging coming from a custom attribute?

How do we enable arbitrary paging support for unknown API paging mechanism?

Looks like deltalink is missing as well.

@calebkiage
Copy link
Contributor Author

Looks like deltalink is missing as well.

@MIchaelMainer, what is deltalink? This was designed to mirror the autorest extension Autorest x-ms-pageable

@baywet
Copy link
Member

baywet commented May 18, 2022

@MIchaelMainer the delta link is a mechanism of sync, not paging. By definition when your application gets a delta link, it's done paging the current changes with the next link, and needs to persist it and come back with it later. I don't think this applies to the scope of these changes.

@MIchaelMainer
Copy link
Member

@baywet Delta query is a sync mechanism. The deltalink can be an end artifact of a nextlink driven paged result set that was started with a delta query. A deltalink can also be used to make a request that results in a paged result set that is driven by nextlink.

I do agree that it is outside the scope of x-ms-pageable and this PR.

@calebkiage calebkiage marked this pull request as ready for review May 27, 2022 16:06
@calebkiage calebkiage requested a review from baywet May 27, 2022 16:07
@calebkiage calebkiage requested a review from baywet May 27, 2022 17:29
@calebkiage calebkiage requested a review from andrueastman June 2, 2022 19:36
@sonarcloud
Copy link

sonarcloud bot commented Jun 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

88.9% 88.9% Coverage
0.0% 0.0% Duplication

@calebkiage calebkiage merged commit 0ff6130 into main Jun 4, 2022
@calebkiage calebkiage deleted the feat/paging branch June 4, 2022 11:06
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.

Add x-ms-pageable OpenAPI Extension
4 participants