search

mehrancodes / laravel-harbor

a cli tool to Quickly create on-demand preview environment for your app on Laravel Forge.
https://www.laravel-harbor.com
MIT License
75 stars 7 forks source link

Error on config value timeout_seconds on string values #37

Closed mehrancodes closed 11 months ago

mehrancodes commented 11 months ago

Issue: When setting the timeout_seconds either from the .env file or within the GitHub workflow, an error message is returned: Invalid configuration values: The timeout seconds field must be a string.

Suggestion: It would be beneficial to allow the class property to accept both string and int types. Subsequently, we can validate the value to ensure it's an integer starting from minimum 0 for safety.

mehrancodes commented 11 months ago

@mgkimsal FYI there was a small bug I faced today while provisioning by using the default integer value for the timeout_seconds 🏋️‍♀️⚡️ Hope it works correctly for you as well

mgkimsal commented 11 months ago

@mehrancodes does the validator coerce in to integer? I would agree, would be better as integer, but coming in from env var, it's likely a string, and I couldn't see a way to coerce it simply without messing with more of the assignment block.

If this works, great.

Thanks.

mehrancodes commented 11 months ago

It seems .env sets it as a string, even though we expect an integer. To support both the config file and .env, I had to accept both types and ensure the value is numeric (either it is string or integer) while validating it. 🤓