chore: development SDK generation script patch

[GBirkel]

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.

[sourcery-ai]

Hey @GBirkel - I've reviewed your changes - here's some feedback:

• Add a --help (or -h) option and a usage message so users can discover the new --scicatlive flag and understand required arguments.
• Rather than hard-coding a binary --scicatlive switch, consider exposing the backend base URL via an environment variable or flag for more flexible use cases.
• Wrap the generated output path ("@scicatproject/scicat-sdk-ts-angular") in quotes when checking for its existence to avoid issues with special characters in directory names.

Prompt for AI Agents

Please address the comments from this code review:
## Overall Comments
- Add a --help (or -h) option and a usage message so users can discover the new --scicatlive flag and understand required arguments.
- Rather than hard-coding a binary --scicatlive switch, consider exposing the backend base URL via an environment variable or flag for more flexible use cases.
- Wrap the generated output path ("@scicatproject/scicat-sdk-ts-angular") in quotes when checking for its existence to avoid issues with special characters in directory names.

## Individual Comments

### Comment 1
<location> `scripts/generate-nestjs-sdk.bash:29` </location>
<code_context>
+
+echo -e "\nFetching the API from local instance of back end..."
+
+if [ $USING_SCICAT_LIVE -eq 1 ];
+then
+	# For when developing with SciCat Live:
</code_context>

<issue_to_address>
Consider computing the API endpoint once and reusing it to simplify the script.

```bash
# instead of two curl calls, compute the API endpoint once…
API_URL=$(
  if [[ $USING_SCICAT_LIVE -eq 1 ]]; then
    echo "http://backend.localhost/explorer-json"
  else
    echo "http://host.docker.internal:3000/explorer-json"
  fi
)

echo -e "\nFetching the API from $API_URL"
curl "$API_URL" -o local-api-for-generator.json

# …and reuse it in the docker invocation
docker run --rm \
  --add-host host.docker.internal:host-gateway \
  -v "$(pwd):/local" \
  openapitools/openapi-generator-cli:v7.13.0 generate \
  -i "$API_URL" \
  -g typescript-angular \
  -o local/@scicatproject/scicat-sdk-ts-angular \
  --additional-properties=ngVersion=19.0.0,npmName=@scicatproject/scicat-sdk-ts-angular,supportsES6=true,withInterfaces=true \
  --skip-validate-spec
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[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

[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

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?

[nitrosx]

Once the requested changes are addressed, I will approve it.
Thanks for helping out

[nitrosx]

Changes to this file are also included in this PR https://github.com/SciCatProject/frontend/pull/1910/files#top.
I left a comment there also, but just in case I leave them here also.

My apologies for being direct, but may ask you to submit cleaner PRs and make sure that changes are not duplicated across PRs?

[GBirkel]

Yeah, that snuck in there because I needed that code on my branch in order to develop with SciCatLive. It shouldn't have been committed. Once it's more complete, I can re-make the PR if necessary.

[nitrosx]

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.

[GBirkel]

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.

[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? 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)

[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?

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?