frontend icon indicating copy to clipboard operation
frontend copied to clipboard

chore: development SDK generation script patch

Open GBirkel opened this issue 5 months ago • 7 comments

The front end repo contains a script: ./scripts/generate-nestjs-sdk.bash It runs the OpenAPI API Generator inside a docker container. It's for when you're making API changes in the back end and want to develop with those changes in the front end.

The generator assumes you're already running a build of the back end on your local machine, and calls it, asking for the API spec. Then the script builds the result, and edits your local node module cache, replacing the "official" @scicatproject/scicat-sdk-ts-angular with the substitute version.

It's weird, but it's convenient: You don't need to upload a package to a repository.

Unfortunately, it didn't work with SciCat Live, because there's no way to map the "backend.localhost" name generated by the proxy into the docker container. So I modified the script to fetch the spec with a curl command into a file, and give the file to the OpenAPI Generator instead of giving it a URL. Easy fix. Since SciCatLive runs the back end at a different address, I added a command-line flag for invoking the script: --scicatlive.

GBirkel avatar Jul 24 '25 16:07 GBirkel

I prefer not to include any service-specific dependent code in the Scicat core, and instead provide a more generic solution. In this case, allowing users to provide arbitrary URLs might be better, as it would help avoid future requests to add new URLs

Junjiequan avatar Jul 31 '25 10:07 Junjiequan

I prefer not to include any service-specific dependent code in the Scicat core, and instead provide a more generic solution. In this case, allowing users to provide arbitrary URLs might be better, as it would help avoid future requests to add new URLs

I agree that this would be a cleaner implementation, but on the other hand, we're talking about a developers-only support script. It should work out-of-the-box as much as possible.

I could modify it to accept an arbitrary URL, but that would also mean adding documentation to SciCatLive somewhere explaining that a specific, always the same, URL should be passed to this script on the command line when invoking. Why make developers go to the trouble? Are we supporting SciCatLive (one of our own projects), or are we not?

GBirkel avatar Aug 04 '25 18:08 GBirkel

I agree that this would be a cleaner implementation, but on the other hand, we're talking about a developers-only support script. It should work out-of-the-box as much as possible.

I could modify it to accept an arbitrary URL, but that would also mean adding documentation to SciCatLive somewhere explaining that a specific, always the same, URL should be passed to this script on the command line when invoking. Why make developers go to the trouble? Are we supporting SciCatLive (one of our own projects), or are we not?

I would much prefer a cleaner solution. The change that you propose (and elaborated further in my comments) is the most sensible.

Regarding SciCatLive, it is an official SciCat supported project, but it is not part of core... at least not yet. I would argue that any scripts in core (FE or BE) should be completely agnostic to SciCatLive, although I agree with you regarding passing the same URL to the script when working in SciCatLive.

I suggest to change the script here to accept an arbitrary URL and add a dedicated script in SciCatLive to call this script with the correct URL specific to SciCatLive. That should implement a solution addressing all concerns.

nitrosx avatar Aug 08 '25 11:08 nitrosx

I suggest to change the script here to accept an arbitrary URL and add a dedicated script in SciCatLive to call this script with the correct URL specific to SciCatLive. That should implement a solution addressing all concerns.

That works for me! I'll change this code and create a ticket for SciCatLive, and assign it to myself.

GBirkel avatar Aug 08 '25 19:08 GBirkel

I've just seen this PR from the linked scicatlive issue. Isn't it easiest just to make this URL come from an ENV var? It can default to http://host.docker.internal:3000 and in scicatlive it would be enough to set it to http://backend:3000 (as this script can use internal docker network I think, as it's run by the container which has access to the docker network and not by the browser)

minottic avatar Aug 11 '25 08:08 minottic

I've just seen this PR from the linked scicatlive issue. Isn't it easiest just to make this URL come from an ENV var?

Well, the idea is that a developer runs this to patch the node package cache in their development environment, whenever they need a new set of API definitions to build against. So the question would be, where/when does the environment variable get set, in that workflow?

GBirkel avatar Aug 11 '25 18:08 GBirkel

Hi, @GBirkel, could you re-run the check so this PR can be merged?

Junjiequan avatar Nov 06 '25 13:11 Junjiequan

@GBirkel could you please merge master in your PR? If all the tests passes, I will be happy to merge.

nitrosx avatar Nov 28 '25 13:11 nitrosx