feat: startup API and APP with dsp-tools (DEV-126) #246
Conversation
| dsp-tools stop-api | ||
| ``` | ||
|
|
||
| This command shuts DSP-API down, deletes all Docker volumes, and removes temporary files. |
There was a problem hiding this comment.
deleting docker volumes through such a command can potentially be risky. You could consider having that as a separate command/flag
There was a problem hiding this comment.
I don't understand. I always use make stack-down-delete-volumes to shut down the API. Is that not good?
By the way, thanks for pointing out that I mistakenly echoed make init-db-test-minimal. The command that I execute is without the "minimal".
There was a problem hiding this comment.
not necessarily "not good" - but if you invest time to set up specific test data, then that will be gone if you delete the volumes (and you need to re-initialize the DB). So e.g. if you modify your datamodel in the App, and then you want to export it to json again using dsp-tools, delete-volumes would not be what you want.
knora/dsplib/utils/start-api.sh
Outdated
| set -e # exit if any statement returns a non-true return value (https://www.davidpashley.com/articles/writing-robust-shell-scripts/) | ||
|
|
||
| # only allow to run this command if we are on a Mac, and if Docker is running. | ||
| [[ ! "$OSTYPE" == "darwin"* ]] && printf "\e[31mERROR: This command can only be run on a Mac. You seem to use another OS.\e[0m\n" && return |
There was a problem hiding this comment.
this seems aggressively restrictive - there is no real reason to disallow this under Linux/WSL if a user wants to do that. And if it doesn't work, it's not our problem anyways ;-)
| @@ -204,6 +221,12 @@ def program(user_args: list[str]) -> None: | |||
| excel2xml(datafile=args.datafile, | |||
| shortcode=args.shortcode, | |||
| default_ontology=args.default_ontology) | |||
| elif args.action == 'start-api' and not sys.platform.startswith('win'): | |||
There was a problem hiding this comment.
This is probably a better way to check the OS, I guess?
There was a problem hiding this comment.
not sure how that would behave with WSL... but probably you shouldn't worry about that too much. So this looks good to me
resolves DEV-126