Improvements in the codebase- Backend converted to typescript #37

Sayan4444 · 2025-02-18T12:47:20Z

This is a part of the issue #13 and split version of PR #29.

Here the backend code is converted from javascript to typescript.

volcano-sh-bot · 2025-02-18T12:47:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign kevin-wangzefeng
You can assign the PR to them by writing /assign @kevin-wangzefeng in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sayan4444 · 2025-02-18T12:47:55Z

/assign @kevin-wangzefeng

Sayan4444 · 2025-03-20T19:32:21Z

Hi @Monokaix , I have added two new things

A new script to automatically generate the types from the CRD
A CI to check if both the frontend and backend builds are correct or not

Its the extension of #65, the part which u mentioned
As you can see the frontend build fails and it has detected it too. If I did break something in the process, would love to correct it. Hope this will be helpful :)

Monokaix · 2025-03-21T01:41:09Z

backend/src/types/queue.d.ts

@@ -0,0 +1,131 @@
+/**


Are these files just an API description or sdk? Can we import to use them?

It is a typescript declaration file and not a API description or sdk and yes we can import them to use them like I did in backend/src/types/index.ts

I WILL TRY MY BEST TO EXPLAIN WHY WE NEED THEM
Unlike statically typed languages like golang where the structure of data is always defined, javascript is a dynamically typed langauge (like python). This behavior introduces a lot of errors which can only be resolved on runtime. Typescript tries to make javascript a statically typed language.

Now when we fetch standard Kubernetes resources like pods,namespaces the type(structure of data) definition of the resource is already defined by the "@kubernetes/client-node" but when we fetch a custom resource, its type is not defined in the "@kubernetes/client-node" library. So we need to manually define it which is done in that files
backend/src/types/queue.d.ts
backend/src/types/job.d.ts

This approach of automated generation of types from standard resource definition is also followed in the "@kubernetes/client-node" (they are using some other library) thus for custom resources definition I tried to do the same.

So can we directly refer to the fields of custom resources after this generation?

Yes, we are just extracting the types out of yaml of the CRD

It is a typescript declaration file and not a API description or sdk and yes we can import them to use them like I did in backend/src/types/index.ts

I WILL TRY MY BEST TO EXPLAIN WHY WE NEED THEM Unlike statically typed languages like golang where the structure of data is always defined, javascript is a dynamically typed langauge (like python). This behavior introduces a lot of errors which can only be resolved on runtime. Typescript tries to make javascript a statically typed language.

Now when we fetch standard Kubernetes resources like pods,namespaces the type(structure of data) definition of the resource is already defined by the "@kubernetes/client-node" but when we fetch a custom resource, its type is not defined in the "@kubernetes/client-node" library. So we need to manually define it which is done in that files backend/src/types/queue.d.ts backend/src/types/job.d.ts

This approach of automated generation of types from standard resource definition is also followed in the "@kubernetes/client-node" (they are using some other library) thus for custom resources definition I tried to do the same.

Hello @Sayan4444,
sorry to say, it could be perceived as overly defensive or aggressive in tone—especially with parts like "I WILL TRY MY BEST TO EXPLAIN WHY WE NEED THEM" in all caps, which can come off as shouting.

Monokaix · 2025-03-21T01:42:40Z

backend/src/types/generate.sh

+check_command jq
+check_command openapi-typescript
+
+kubectl get crd jobs.batch.volcano.sh -o json | jq '{


Is there another way to generate instead of kubectl? Becasue this need user create a real cluster.
How about just generate through yaml or some static files?

Yes we can generate it throught static yaml
https://github.com/volcano-sh/volcano/tree/master/config/crd/volcano/bases

But what I have understood is to have this application started or to make the api calls, its mandatory to have volcano deployed in the kubernetes cluster. So thats why I thought why not to use something which is most readily available.

There is also an API repo https://github.com/volcano-sh/apis

Hi sir, the types only need to be generated when there are changes in the CRD which is rare i.e. we dont need to always use "generate.sh" while setting up the project and only use it if the types are invalid

To generate the types of a resource, we need openapi-spec file of that resource. In libraries like https://github.com/kubernetes-client/javascript
the openapi-spec file of the native resources are already cached making the type generation faster. In our case we are fetching the openapi-spec it from the cluster and getting the types from it.

WHY FETCHING FROM CLUSTER INSTEAD OF CACHING?
We only generate the types (i.e. use generate.sh) when the CRD changes. A updated CRD means a modified openapi-spec file making the old one invalid.
So now since the old types are invalid we need to generate the new types thus we need new openapi spec file. This exact step is automated via this script.

I hope am able to convey my point 😊

I think we'd better add a doc to let users know how to use this script.

Hi @Monokaix, I added the docs under
docs/generateScript.md

karanBRAVO

@Monokaix @Sayan4444

karanBRAVO · 2025-03-21T12:23:31Z

.github/workflows/build.yml

I think we should add this workflow after adding tests. #53

karanBRAVO · 2025-03-21T12:27:33Z

backend/tsconfig.json

I think we should remove unnecessary commented code.
Also, it would be great to add import alias @ so that we do not have to do ../../path/to/file

Removed the commented code

volcano-sh-bot · 2025-03-21T12:33:26Z

@karanBRAVO: changing LGTM is restricted to collaborators

In response to this:

@Monokaix @Sayan4444

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Monokaix · 2025-04-07T09:23:18Z

docs/generateScript.md

+## Prerequisites
+Before running the script, ensure the following tools are installed and properly configured on your system:
+- **kubectl**: Required to interact with the Kubernetes cluster and fetch the OpenAPI specification.
+- **pq**: A utility used for processing the fetched data.


Maybe we should also add note that a ready cluster with volcano installed?

jq?

corrected it, it was a typo

Monokaix · 2025-04-07T09:23:39Z

docs/generateScript.md

@@ -0,0 +1,34 @@
+# Script Explanation: `generate.sh`


Maybe we can give an absolute path.

Changed it. Actually, since the same types are used for the frontend also, so in future it will a general script. For now it only affects the backend, if approved will generalise it "types" of both frontend and backend

de6p · 2025-04-07T09:24:48Z

@Monokaix this PR also add ts support.

Monokaix · 2025-04-07T09:27:50Z

docs/generateScript.md

+    npm run generate:types
+    ```
+This will fetch the latest OpenAPI specification and generate the TypeScript types for the custom resources.
+## Purpose


Maybe we should emphasize that here we just generate volcano related resources to develop codes.

Shrutim1505 · 2025-04-07T09:30:31Z

@Sayan4444 is your backend conversation to typescript complete, since I started working on backend now so it might create conflicts with my existing changes.
Is ur PR mergable?

Sayan4444 · 2025-04-07T09:36:04Z

@Sayan4444 is your backend conversation to typescript complete, since I started working on backend now so it might create conflicts with my existing changes. Is ur PR mergable?

Yes my PR has no conflicts with the current base branch

Monokaix · 2025-04-07T09:37:32Z

Please squash to one commit and make ci happy.

Sayan4444 · 2025-04-07T13:03:07Z

Hi @Monokaix, I have updated the readme. If this gets approved I will squash

Shrutim1505 · 2025-04-07T14:27:53Z

Hi @Sayan4444 as of now ,readme can be updated afterwards, u complete this work first 🙃, because it is pausing work of other contributers on backend

Signed-off-by: Sayan Banerjee <[email protected]>

Sayan4444 · 2025-04-07T18:06:13Z

@Shrutim1505 Sorry for that and my work is completed.

Sayan4444 · 2025-04-07T18:07:09Z

Please squash to one commit and make ci happy.

I have squashed it into a single commit. The ci still fails cause the frontend is broken.

Shrutim1505 · 2025-04-07T18:20:20Z

Once more after squashing can u please run Ci check

Sayan4444 · 2025-04-07T18:24:11Z

@Shrutim1505 For every new push the ci runs again only

volcano-sh-bot requested review from kevin-wangzefeng and william-wang February 18, 2025 12:47

volcano-sh-bot added the size/XXL label Feb 18, 2025

volcano-sh-bot assigned kevin-wangzefeng Feb 18, 2025

Sayan4444 force-pushed the backend-typescript branch 4 times, most recently from c4b87bd to 70e36f1 Compare March 20, 2025 19:26

Monokaix reviewed Mar 21, 2025

View reviewed changes

karanBRAVO suggested changes Mar 21, 2025

View reviewed changes

Sayan4444 mentioned this pull request Mar 25, 2025

Request to be a member of Volcano community volcano-sh/community#73

Open

Monokaix mentioned this pull request Apr 7, 2025

added necessary packages #75

Open

Monokaix reviewed Apr 7, 2025

View reviewed changes

backend converted to typescript

6f064fe

Signed-off-by: Sayan Banerjee <[email protected]>

Sayan4444 force-pushed the backend-typescript branch from 74e28e2 to 6f064fe Compare April 7, 2025 18:03

Improvements in the codebase- Backend converted to typescript #37

Are you sure you want to change the base?

Improvements in the codebase- Backend converted to typescript #37

Conversation

Sayan4444 commented Feb 18, 2025

volcano-sh-bot commented Feb 18, 2025

Sayan4444 commented Feb 18, 2025

Sayan4444 commented Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Sayan4444 Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sayan4444 Mar 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karanBRAVO left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

volcano-sh-bot commented Mar 21, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

de6p commented Apr 7, 2025

Choose a reason for hiding this comment

Shrutim1505 commented Apr 7, 2025

Sayan4444 commented Apr 7, 2025

Monokaix commented Apr 7, 2025

Sayan4444 commented Apr 7, 2025

Shrutim1505 commented Apr 7, 2025

Sayan4444 commented Apr 7, 2025

Sayan4444 commented Apr 7, 2025

Shrutim1505 commented Apr 7, 2025

Sayan4444 commented Apr 7, 2025

Sayan4444 commented Mar 20, 2025 •

edited

Loading

Sayan4444 Mar 21, 2025 •

edited

Loading

Sayan4444 Mar 25, 2025 •

edited

Loading