-
Notifications
You must be signed in to change notification settings - Fork 32
POC: Generating OpenAPI client #1213
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
base: main
Are you sure you want to change the base?
POC: Generating OpenAPI client #1213
Conversation
@@ -173,6 +174,11 @@ async def update_user_setting(setting: UserSetting, user: User = Depends(authent | |||
from ee.api import ai_optimisations | |||
app.include_router(ai_optimisations.router) | |||
|
|||
for route in app.routes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see c9e1aed
@@ -77,7 +79,8 @@ const fetchAndFillRunData = async (url_params) => { | |||
let run = null; | |||
|
|||
try { | |||
run = await makeAPICall('/v2/run/' + url_params['id']) | |||
const api = new DefaultApi(); | |||
run = await api.getRun(url_params['id']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the relevant improvement. when using TypeScript, there could even be validation for parameters and responses on the client side
|
||
openapi-generator-cli generate \ | ||
--generator-name javascript \ | ||
--input-spec http://127.0.0.1:8000/openapi.json \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of using the backend port, a local file can be used
This PR feels a bit chaotic and I am trying to understand what the purpose is:
|
sorry about that! it was not meant to be a standalone changeset but only an additional proof of concept to highlight what we had talked about. I've added a proper description now.
it uses a library (https://openapi-generator.tech/) to generate code from the specification provided by FastAPI. the bash script is just for documenting the command line flags that I have used. most of them could be added to a config file. whether or not to keep a script as a shortcut for the generator command is a matter of taste. the command (or script) needs to be run whenever the backend API changes.
that is true and most of it is due to the fact that OpenAPI is strictly typed while JavaScript is not. there is much less boilerplate when using TypeScript (but that would be a separate discussion). some of the boilerplate can be avoided by fine tuning the config and there is also an open feature request to skip client side validation and rely on server side validation instead: OpenAPITools/openapi-generator#4168
I think I have answered most of the question above. whether to add the (redundant) client code to the repository or not is a highly subjective matter. personally I prefer to do so because then you can search in it or run git blame without navigating to the backend (essentially avoiding context switches). |
This is a proof of concept for replacing the existing request logic in the frontend with a client implementation that is automatically generated from the OpenAPI specification. It is not meant to be merged but only serves to demonstrate the concept.
Advantages
Disadvantages