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.

Sign up for GitHub

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

Jump to bottom

Add project-specific startup script support (eg. for cron configuration) #263

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add project-specific startup script support (eg. for cron configuration) #263

wants to merge 1 commit into from

Conversation

martin-rueegg
Copy link

Allow user to add *.sh files to $HOST_PATH_HTTPD_DATADIR/*/.devilbox/autostart which will then be executed on startup of the php container.

@martin-rueegg martin-rueegg mentioned this pull request Dec 16, 2022
Open
@martin-rueegg martin-rueegg force-pushed the add-project-startup-script-support branch from d35d8e5 to 06b3a94 Compare December 16, 2022 13:39
@martin-rueegg martin-rueegg mentioned this pull request Dec 16, 2022
Closed
@martin-rueegg martin-rueegg force-pushed the add-project-startup-script-support branch from 06b3a94 to bc65179 Compare December 16, 2022 17:27
@martin-rueegg martin-rueegg mentioned this pull request Dec 16, 2022
Open
cytopia
cytopia reviewed Jan 2, 2023
View reviewed changes
Copy link
Member

@cytopia cytopia left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@martin-rueegg

This image already allows for adding "custom" scripts, but adding them hard-coded would not work as expected (see specific comment).

As an alternative, which would make more sense and is more flexible, I would add this directly into the Devilbox as an example script. Similar to what has been done here in the autostart/ dir for blackfire and nodejs:

https://github.com/cytopia/devilbox/tree/master/autostart

Dockerfiles/prod/data/docker-entrypoint.d/311-project-startup-scripts.sh Outdated

for DEVILBOX_PROJECT_DIR in /shared/httpd/*; do

script_dir=${DEVILBOX_PROJECT_DIR}/.devilbox/autostart
Copy link
Member

Choose a reason for hiding this comment

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

The PHP-FPM images are not aware about the .devilbox/ directory. This is specific to the HTTPD images and can also be changed to any other directory:

https://github.com/devilbox/docker-nginx-mainline/blob/master/doc/environment-variables.md#-mass_vhost_template_dir

So hardcoding it here would not work.

In other words, the PHP-FPM images don't know about this path and hardcoding it here would result in it not working, once you change it via the HTTPD images (or in our use-case vie the Devilbox .env variable)

Copy link
Author

Choose a reason for hiding this comment

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

I see.

Would it be an option to propagate the MASS_VHOST_TEMPLATE_DIR to the PHP-FPM images too and use it in the script? Or create a new variable that defaults to the same directory?

Copy link
Member

Choose a reason for hiding this comment

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

Would it be an option to propagate the MASS_VHOST_TEMPLATE_DIR

This would not make much sense, as the PHP images have nothing to do with it.

Copy link
Author

Choose a reason for hiding this comment

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

@cytopia Actually, You are already populating the MASS_VHOST_TEMPLATE_DIR to the php image in the form of HTTPD_TEMPLATE_DIR and you're even using it yourself in your code for the status page.

I have updated the code in order to use the value of that variable.

@martin-rueegg
Copy link
Author

As an alternative, which would make more sense and is more flexible, I would add this directly into the Devilbox as an example script. Similar to what has been done here in the autostart/ dir for blackfire and nodejs:

https://github.com/cytopia/devilbox/tree/master/autostart

The disadvantage of the global autostart directory is that it is not project-specific and hence not distributed with the project code (e.g. via version control). It requires every developer working on the project to run additional installation steps for the project.

@cytopia
Copy link
Member

cytopia commented Jan 27, 2023

As an alternative, which would make more sense and is more flexible, I would add this directly into the Devilbox as an example script. Similar to what has been done here in the autostart/ dir for blackfire and nodejs:
https://github.com/cytopia/devilbox/tree/master/autostart

The disadvantage of the global autostart directory is that it is not project-specific and hence not distributed with the project code (e.g. via version control). It requires every developer working on the project to run additional installation steps for the project.

The global autostart (all versions or per version) offers exactly this functionality. You can add any kind of shell script. It is just a generic way to start things up. What you do with it is totally up to the user.

What I can think of, is to add some generic template scripts *-example or add this to the documentation.

@martin-rueegg martin-rueegg force-pushed the add-project-startup-script-support branch from bc65179 to d0e6ea4 Compare February 23, 2023 17:35
@martin-rueegg martin-rueegg force-pushed the add-project-startup-script-support branch from d0e6ea4 to b75c0eb Compare February 23, 2023 17:43
@martin-rueegg
Copy link
Author

As an alternative, which would make more sense and is more flexible, I would add this directly into the Devilbox as an example script. Similar to what has been done here in the autostart/ dir for blackfire and nodejs:
https://github.com/cytopia/devilbox/tree/master/autostart

The disadvantage of the global autostart directory is that it is not project-specific and hence not distributed with the project code (e.g. via version control). It requires every developer working on the project to run additional installation steps for the project.

The global autostart (all versions or per version) offers exactly this functionality. You can add any kind of shell script. It is just a generic way to start things up. What you do with it is totally up to the user.

What I can think of, is to add some generic template scripts *-example or add this to the documentation.

It would still require to deploy the autostart script.

With the updated version of the code, the vHost config dir MASS_VHOST_TEMPLATE_DIR /HTTPD_TEMPLATE_DIR is used.

Does that make it better suitable or are you against any built-in support for this (as opposed to the global autostart scripts?

@martin-rueegg martin-rueegg reopened this Feb 23, 2023
@martin-rueegg martin-rueegg force-pushed the add-project-startup-script-support branch 2 times, most recently from 9b057b5 to b5c2522 Compare February 23, 2023 17:59
@martin-rueegg
Add project-specific startup script support (e.g. for cron configurat…
9ab9ae3 
…ion)

Allow user to add *.sh files to  `$HOST_PATH_HTTPD_DATADIR/*/$HTTPD_TEMPLATE_DIR/autostart` which will then be executed on startup of the php container.
@martin-rueegg martin-rueegg force-pushed the add-project-startup-script-support branch from b5c2522 to 9ab9ae3 Compare February 23, 2023 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cytopia cytopia cytopia left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@martin-rueegg @cytopia