Skip to content

Conversation

@Haigutus
Copy link

Copy link
Contributor

@Tristan-WorkGH Tristan-WorkGH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @Haigutus !
Thanks you for your PR.

It's a good idea to have a script to simplify the setup, here first thoughts I have:

Comment on lines +18 to +19
GRIDSUITE_DATABASES=$(realpath "DATABASES")
export GRIDSUITE_DATABASES
Copy link
Contributor

@Tristan-WorkGH Tristan-WorkGH Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't support a "DATABASES" env var, we define directly "GRIDSUITE_DATABASES" in our env (.bashrc for example).
It's up to users to decide how they manage it in their environment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here it creates default folder DATABASES, it is not an ENV variable, the ENV variable is still "GRIDSUITE_DATABASES". One could add check, if it is already defined, then it will not be set again

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we prefer to create GRIDSUITE_DATABASES env only if it does not exist

Copy link
Contributor

@Tristan-WorkGH Tristan-WorkGH Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We at first decided to not do scripts like this because there is in day-to-day too much cases to support and we don't want to have to maintain these scripts.

I will ask our TL.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • One still needs to maintain the same info in English text form in setup guide.
  • This would also help to test the setup, in case of changes to how the clean setup is to be made
  • Not all variations need to be supported just one for testing and as an example (one can modify it, instead of staring to write on based on the setup guide again)

Comment on lines +18 to +19
GRIDSUITE_DATABASES=$(realpath "DATABASES")
export GRIDSUITE_DATABASES
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we prefer to create GRIDSUITE_DATABASES env only if it does not exist


# Assuming the Docker Compose file is relative to the script's execution path
cd docker-compose/explicit-profiles || { echo "Failed to change directory. Check if the path is correct. Exiting."; exit 1; }
docker compose --profile suite up -d || { echo "Docker Compose failed to start. Check Docker setup. Exiting."; exit 1; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer to pass the profile name in a parameter to start and stop

Kibana and Grafana to listen external traffic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants