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

Drop hardcoded /application path #20

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

Conversation

kdambekalns
Copy link
Member

The handling of .warmupdone & beach-cron-hourly.sh used an hardcoded path instead of whatever BEACH_APPLICATION_PATH was set to.

The handling of `.warmupdone` & `beach-cron-hourly.sh` used an
hardcoded path instead of whatever `BEACH_APPLICATION_PATH` was set
to.
@kdambekalns
Copy link
Member Author

Hm… This probably needs an adjustments to whereever .warmupdone is actually used (health check, …) – or we leave that as is?

@robertlemke
Copy link
Member

You can make the path leading to .warmupdone configurable as you did, if you also make the configuration of the health check for instance pods more flexible. Problem is that if you configure the APPLICATION_PATH variable as an instance variable, there's no easy way to access it in the code which generates the pod configuration:

https://github.com/flownative/beach-controlpanel/blob/dfe8544657c49605dae9d3f71aa9e8502072f040/DistributionPackages/Flownative.Beach/Classes/Domain/Context/Instance/Service/InstanceKubernetesService.php#L1069

We could make the application path a "first-class-citizen", or more specifically, a property of an instance. Currently there's a concept called "DeploymentFlags" used for that, you might want to check the respective model. There's also the concept of userProvidedConfiguration in those flags, but it is not very elaborate.

In the end, we need to decide if we want to take the extra step and create a way to configure those kinds of variables / properties in a general fashion via the UI or .beach.yaml. Or if we find a workaround.

Another approach might be to make the health check endpoints configurable.

@kdambekalns kdambekalns force-pushed the bugfix/drop-hardcoded-application-path branch from 6f26923 to 42982e6 Compare September 27, 2024 10:56
@kdambekalns
Copy link
Member Author

A chicken-out solution would be to leave the .warmupdone where it currently is – no harm done. I'll think about it…

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.

2 participants