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

Generate OpenAPI spec using Utoipa #122

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

Generate OpenAPI spec using Utoipa #122

wants to merge 2 commits into from

Conversation

hdoordt
Copy link
Member

@hdoordt hdoordt commented Oct 23, 2024

Very much an experiment as of right now, but this PR aims to:

  • Allow for ergonomically separating Changeset logic from Request/Response payloads
  • Allow for reusing validation logic specified in Changesets
  • Allow for generating OpenAPI spec using utoipa for the Request/Response payloads

@hdoordt
Copy link
Member Author

hdoordt commented Oct 23, 2024

The question right now would be: Is this going in the right direction?

@hdoordt hdoordt force-pushed the utoipa branch 4 times, most recently from 899661c to e84f01f Compare October 29, 2024 08:50
@hdoordt
Copy link
Member Author

hdoordt commented Oct 29, 2024

I think this feature works nicely now and does not hurt ergonomics too much, but I could use some opinions on particularly the ergonomics of the payloads module in the tasks controller in the full blueprint

Copy link
Member

@marcoow marcoow left a comment

Choose a reason for hiding this comment

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

My general feeling is on the one hand side, I can see how this is useful in some cases while on the other hand, it introduces a ton of new concepts and accidental complexity… I'm a bit torn – might well be good to have this but likely opt-in. At the same time, I'm not sure this should be the priority for now when we have a number of other things to figure out first:

  • relations between entities
  • some kind of background job functionality
  • deployment story (might only be documentation but I guess regarding e.g. how to run migrations on production during deployment, we have some blind spots)

@hdoordt
Copy link
Member Author

hdoordt commented Nov 22, 2024

My general feeling is on the one hand side, I can see how this is useful in some cases while on the other hand, it introduces a ton of new concepts and accidental complexity… I'm a bit torn – might well be good to have this but likely opt-in. At the same time, I'm not sure this should be the priority for now when we have a number of other things to figure out first:

  • relations between entities
  • some kind of background job functionality
  • deployment story (might only be documentation but I guess regarding e.g. how to run migrations on production during deployment, we have some blind spots)

@marcoow thanks for the insight. I agree we may need to do some more thinking/experimenting before adding a feature like this. I'll add issues for the things you listed

@hdoordt hdoordt closed this Nov 22, 2024
@marcoow
Copy link
Member

marcoow commented Nov 22, 2024

@hdoordt I wouldn't necessarily suggest to close this – I think it can be useful and we could do it at some point, I just think we'd need to think about how we add this exactly

@hdoordt hdoordt reopened this Nov 22, 2024
@hdoordt
Copy link
Member Author

hdoordt commented Nov 22, 2024

@hdoordt I wouldn't necessarily suggest to close this – I think it can be useful and we could do it at some point, I just think we'd need to think about how we add this exactly

I don't know what happened there, but closing the PR was not intentional. Reopening it

@jondot
Copy link

jondot commented Dec 10, 2024

Sharing some of our experience in Loco, it seems like OpenAPI support is highly sought after, but (using utoipa or not) can introduce a lot of cruft, friction and redundant information that has to be typed manually.

So this echoes @marcoow 's experience.

Personally, I'm still trying to figure out - is it OpenAPI that is needed by intent or "a way to create a client for the API magically".

I believe the key is figuring out the intent for the OpenAPI requirement. I'm still deliberating on this myself.

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.

3 participants