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

DM-47769: Add LinkData model for parsing HTTP Link headers #334

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

rra
Copy link
Member

@rra rra commented Nov 22, 2024

Add safir.models.LinkData, which parses HTTP Link headers for pagination information. This is useful for clients of services that use the database pagination support, including their test suites.

Add `safir.models.LinkData`, which parses HTTP `Link` headers for
pagination information. This is useful for clients of services that
use the database pagination support, including their test suites.
@rra rra requested a review from fajpunk November 22, 2024 23:11
@rra
Copy link
Member Author

rra commented Nov 22, 2024

This little toy model isn't great and doesn't handle the full syntax, but I already had it sitting around and it's enough to use in test suites without having to write a parser. Maybe I should add more disclaimers to the documentation? If we were using requests, we could use its built-in parser, but httpx apparently doesn't have such a thing and there's only one kind-of abandoned Python module that I could find.

Copy link
Member

@fajpunk fajpunk left a comment

Choose a reason for hiding this comment

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

What about calling it PaginationLinkData, or something not so verbose but that still indicates it's only useful for pagination headers? Or, keep it in the safir.database namespace to indicate that its only useful there?

Move `LinkData` to the `safir.database` module, since at present
it's only useful for extracting pagination information in the form
used there, and rename it to `PaginatedLinkData`.
@rra
Copy link
Member Author

rra commented Nov 25, 2024

Ah, yes, both of those changes sound good. Done.

@rra rra enabled auto-merge November 25, 2024 17:48
@rra rra merged commit 50b38d5 into main Nov 25, 2024
5 checks passed
@rra rra deleted the tickets/DM-47769a branch November 25, 2024 17:52
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