Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added start and stop scripts #15

Merged
merged 6 commits into from
Oct 15, 2024
Merged

Added start and stop scripts #15

merged 6 commits into from
Oct 15, 2024

Conversation

ezimuel
Copy link
Collaborator

@ezimuel ezimuel commented Oct 14, 2024

This is a proposal for adding a start.sh and stop.sh scripts to facilitate the start and the stop of the docker services. This script can be used after the start-local installation, e.g. after a reboot. Using these scripts the user do not need to interact with docker commands anymore for start, stop or re-start Elasticsearch and Kibana.

@ezimuel ezimuel added the enhancement New feature or request label Oct 14, 2024
@ezimuel ezimuel requested a review from leemthompo October 14, 2024 15:21
Copy link
Contributor

@leemthompo leemthompo left a comment

Choose a reason for hiding this comment

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

[Docs review] Nice addition! Just some tiiiiiny copyediting nits from me 🤓.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@ezimuel ezimuel requested a review from pquentin October 15, 2024 07:21
ezimuel and others added 3 commits October 15, 2024 09:21
Co-authored-by: Liam Thompson <[email protected]>
Co-authored-by: Liam Thompson <[email protected]>
Co-authored-by: Liam Thompson <[email protected]>
@ezimuel
Copy link
Collaborator Author

ezimuel commented Oct 15, 2024

Thanks @leemthompo for the review!

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! The functionality works. I've added a few nits to try and clarify the output.

start-local.sh Outdated
@@ -475,6 +536,7 @@ print_steps() {
echo "- Generated random passwords"
echo "- Created a .env file with settings"
echo "- Created a docker-compose.yml file"
echo "- Created start/stop/uninstall commands"
Copy link
Member

Choose a reason for hiding this comment

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

Small suggestion on the wording. I'm not sure of it, just thinking out loud here.

Suggested change
echo "- Created start/stop/uninstall commands"
echo "- Created start.sh/stop.sh/uninstall.sh scripts"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think these details are important. The message is to inform the user about the actions and the fact that there are 3 commands that can be used.

start-local.sh Outdated
Comment on lines 536 to 539
echo "- Generated random passwords"
echo "- Created a .env file with settings"
echo "- Created a docker-compose.yml file"
echo "- Created start/stop/uninstall commands"
Copy link
Member

Choose a reason for hiding this comment

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

What are your thoughts on using indentation to clarify the files that go into elastic-start-local?

- Generated random passwords
- Created the elastic-start-local folder with:
  - .env file with settings
  - docker-compose.yml
  - start.sh/stop.sh/uninstall.sh scripts
- Running docker compose up --wait

@ezimuel ezimuel merged commit 5a5fba7 into main Oct 15, 2024
7 checks passed
@ezimuel
Copy link
Collaborator Author

ezimuel commented Oct 15, 2024

Thanks @pquentin for the review!

@ezimuel ezimuel deleted the start-stop branch October 23, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants