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

Fix OpenAPI #715

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Fix OpenAPI #715

wants to merge 7 commits into from

Conversation

chuck-dbos
Copy link
Collaborator

Make independent from example templates
Add v2 decorator support
Fix 3 customer bugs in paths / argument handling
Fix the multiple entrypoint issue from #671

@chuck-dbos chuck-dbos marked this pull request as ready for review January 15, 2025 17:48
@chuck-dbos chuck-dbos requested a review from devhawk January 15, 2025 17:48
Copy link
Member

@kraftp kraftp left a comment

Choose a reason for hiding this comment

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

Does this work with TSv2? The test is only for the contexts API.

@chuck-dbos
Copy link
Collaborator Author

Does this work with TSv2? The test is only for the contexts API.

Yep. See test line 12, 58-101, 266-356, and 488-end.

@kraftp
Copy link
Member

kraftp commented Jan 15, 2025

Does this work with TSv2? The test is only for the contexts API.

Yep. See test line 12, 58-101, 266-356, and 488-end.

Got it! If we're bringing OpenAPI back, we also need to add it to the list of packages in .github/ and renable the decorator in middleware.ts.

@chuck-dbos
Copy link
Collaborator Author

Got it! If we're bringing OpenAPI back, we also need to add it to the list of packages in .github/ and renable the decorator in middleware.ts.

Good point, and I seem to have missed the confluent kafka lib in .github/ also.

@qianl15
Copy link
Member

qianl15 commented Jan 15, 2025

If I understand it correctly, OpenAPI is only useful if you're using DBOS.GetAPI, DBOS.PostAPI, etc? If I don't have any DBOS HTTP endpoints, then nothing matters. I think we can hold this off until after TS V2 launch. Because this launch tomorrow focuses on compatibility with other popular frameworks, plus this PR is huge.

@devhawk
Copy link
Collaborator

devhawk commented Jan 16, 2025

plus this PR is huge.

It's only huge because the OpenAPI stuff was removed in #711. It's less < 100 lines of production code changes + almost 300 lines of new tests.

image

@chuck-dbos chuck-dbos marked this pull request as draft January 16, 2025 13:02
@chuck-dbos
Copy link
Collaborator Author

Marking draft again.
There are two things I'd like to improve prior to re-release:

  1. Change (or at least re-evaluate) where the output goes for multi-file generation
  2. Move the decorator out of core and into the extension package.

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.

4 participants