-
Notifications
You must be signed in to change notification settings - Fork 2
fix: removed old library and added autogenerated one #264
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?
Conversation
Signed-off-by: alexpargon <[email protected]>
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.
Pull Request Overview
This PR replaces an old osparc API client library with an autogenerated one, updating import paths across the codebase from relative imports to the new osparc-api-ts-client package name.
- Updated all import statements from relative paths (
../../osparc-api-ts-client) to the npm package name (osparc-api-ts-client) - Added OpenAPI generator tooling and configuration for automated client generation
- Generated new API client files in the osparc-api-ts-client directory
Reviewed Changes
Copilot reviewed 154 out of 172 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| node/package.json | Added OpenAPI generator dependency and npm script for client generation |
| node/openapitools.json | Added OpenAPI generator configuration for TypeScript client generation |
| node/src/osparc-api-ts-client/* | Generated TypeScript API client files (models, APIs, utilities) |
| node/src/context/* | Updated import paths to use npm package name instead of relative paths |
| node/src/components/* | Updated import paths to use npm package name instead of relative paths |
Files not reviewed (2)
- node/package-lock.json: Language not supported
- node/src/osparc-api-ts-client/package-lock.json: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Wrt to the above, this will have to be fixed by using the correct types in the flaskapi/frontend. |
|
and wrt the 'is possibly undefined' errors etc. Yeah, some of these fields are nullable, so you will have to be able to handle that case in your code. |
Signed-off-by: alexpargon <[email protected]>
|
@alexpargon what is the status of this? Also, if adding the api-files to gitignore, then please include the generation command in the Makefile or elsewhere - otherwise it will only work in your directory and building will fail for the CI or any other dev. |
The generation is on the package.json file, just run the |
|
Now this branch compiles and works, but still requires proper testing, as something can be broken with so many changes. the i finally added also the JRE installation process on the dockerfile for the Node build process and it compiles as it should, only testing remains |
Signed-off-by: alexpargon <[email protected]>
Signed-off-by: alexpargon <[email protected]>
Yeah, i have a feeling that you might want to hold this one off until after the release, we don't have much time anymore to fix things. |
|
Well, I am also changing the backend extensively, including error propagation - and if there are breaking changes in the backend, we will encounter them whether we update the client's reference API or not (NB this is just updating the types for TypeScript to reference, all real interactions happen through Python) |
|
in any case this is done, so we can either merge it after testing it or wait for it after release |
|
I was thinking, maybe we want to apply the same hold-on logic to my branch as well. We could create a "release" branch to which we only merge changes that we consider safe, and focuses mostly on UI/UX features by @alexpargon . We can keep merging the more risky stuff in main, keep merging release into it, and only merge main into release once we are reasonably sure of its stability. @wvangeit what do you think? |
This is normally called a development branch:
|
|
In general I'm ok with a development branch, or even in favor. |

these changes still need work due to missing properties on the backend definition of the api surfaces:
they need to be fixed before this PR will work