Skip to content

feat: convert datasetio API to use a FastAPI router #4400

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.

Sign up for GitHub

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

Jump to bottom
Open
nathan-weinberg wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: convert datasetio API to use a FastAPI router #4400

nathan-weinberg wants to merge 1 commit into from
+518 −166

Conversation

@nathan-weinberg
Copy link
Contributor

@nathan-weinberg nathan-weinberg commented Dec 16, 2025

What does this PR do?

Migrate from @webmethod decorators to FastAPI router pattern

Closes #4343

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Dec 16, 2025
leseb
leseb requested changes Dec 17, 2025
View reviewed changes
Copy link
Collaborator

@leseb leseb left a comment

Choose a reason for hiding this comment

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

minor nit otherwise LGTM

@mergify
Copy link

mergify bot commented Dec 17, 2025

This pull request has merge conflicts that must be resolved before it can be merged. @nathan-weinberg please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Dec 17, 2025
@mergify mergify bot removed the needs-rebase label Dec 17, 2025
cdoern
cdoern approved these changes Dec 17, 2025
View reviewed changes
Copy link
Collaborator

@cdoern cdoern 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

cdoern
cdoern requested changes Dec 17, 2025
View reviewed changes
Copy link
Collaborator

@cdoern cdoern left a comment

Choose a reason for hiding this comment

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

hold on, there is a stainless conflict.

cdoern
cdoern approved these changes Dec 17, 2025
View reviewed changes
Copy link
Collaborator

@cdoern cdoern left a comment

Choose a reason for hiding this comment

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

stainless passing now

@ashwinb
Copy link
Contributor

ashwinb commented Dec 17, 2025

image

why is this new warning emitted now?

@nathan-weinberg
Copy link
Contributor Author

nathan-weinberg commented Dec 18, 2025

why is this new warning emitted now?

@cdoern any idea why? i know you have to do some manual work to get stainless working before

happy to make any code changes to the PR if needed

@nathan-weinberg nathan-weinberg force-pushed the fastapi-datasetio branch 2 times, most recently from 5477c5f to 8543981 Compare December 18, 2025 20:36
@nathan-weinberg nathan-weinberg force-pushed the fastapi-datasetio branch 3 times, most recently from 24730d2 to da6ccf9 Compare January 5, 2026 15:39
@mergify
Copy link

mergify bot commented Jan 6, 2026

This pull request has merge conflicts that must be resolved before it can be merged. @nathan-weinberg please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Jan 6, 2026
@mergify mergify bot removed the needs-rebase label Jan 6, 2026
@nathan-weinberg nathan-weinberg force-pushed the fastapi-datasetio branch 2 times, most recently from 920d76c to dc4c5e5 Compare January 6, 2026 20:12
@llamastack llamastack deleted a comment from github-actions bot Jan 7, 2026
@leseb
Copy link
Collaborator

leseb commented Jan 7, 2026

stainless job is not updating the comment, I tried to remove it, we can try to repush

@cdoern
Copy link
Collaborator

cdoern commented Jan 7, 2026

the stainless job is disabled for now which is why its not updating

@nathan-weinberg
feat: convert datasetio API to use a FastAPI router
e38e24a 
Migrate from @webmethod decorators to FastAPI router
pattern

Signed-off-by: Nathan Weinberg <nweinber@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cdoern cdoern cdoern approved these changes

@ashwinb ashwinb Awaiting requested review from ashwinb ashwinb is a code owner

@raghotham raghotham Awaiting requested review from raghotham raghotham is a code owner

@ehhuang ehhuang Awaiting requested review from ehhuang ehhuang is a code owner

@bbrowning bbrowning Awaiting requested review from bbrowning bbrowning is a code owner

@mattf mattf Awaiting requested review from mattf mattf is a code owner

@franciscojavierarceo franciscojavierarceo Awaiting requested review from franciscojavierarceo franciscojavierarceo is a code owner

@leseb leseb Awaiting requested review from leseb leseb is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Convert datasetio API to use a FastAPI router

4 participants

@nathan-weinberg @ashwinb @leseb @cdoern