laravel-aws-worker
laravel-aws-worker copied to clipboard
dusterio
Removed use of env function to support cached environments
This Pull Request removes the use of the env() function to obtain the value of the "REGISTER_WORKER_ROUTES" environment variable.
Is related to the this issue.
This pull request breaks compatibility with previous versions. Users who want to install a new version with this code will have to publish the package in order to make it work as it has been working until now.
Any chance of getting this merged in?
Laravel Docs:
If you execute the config:cache command during your deployment process, you should be sure that you are only calling the env function from within your configuration files. Once the configuration has been cached, the .env file will not be loaded and all calls to the env function will return null.
Thanks
@faculezcano could you please give a real-world example of when a config would be cached? I always assumed that you set the environment variable once and pretty much don't have to touch again
Oh i missed the point that im using elastic beanstalk to deploy and the project folder is purged, you're right isnt really needed.
@dusterio See the Laravel documentation about this. env() function should not be used outside the config files.
@jlorente this is correct for basic installations (with .env files). in case of AWS - we are talking brand new containers every time and real environment variables only. Containers are immutable
Maybe you don't want to use the environment variables coming from EB because there is a limit of the environment variables that you can define in EB. In my case, I deploy an .env file with the application because the total size of my variables exceeds the 4,096 bytes limit. In any case, this solution will work with cached and non cached environments.
@jlorente if you are deploying unique files (stateful) in your containers, why not just include a config file with values then? for variables that are longer than 4096 characters.
if I understand correctly the purpose of .env file was local machine development - to simulate real environment variables quickly.
The reason I chose to use env() rather than config file is because I wanted to make this package as little (overhead) as possible. Since it's for stateless containers, it didn't make sense to use config.
But I see your point, I only had to include longer variables with AWS once and what I did is - I broke long strings into multiple, numbered variables, then concatenated them later in the code.
Ok, I understand your point of making the package as little as possible. Mine is a concrete case and I am working with my fork of the project now, so I have no need of merging this pull request. But answering your last point, the problem is not the length of one variable, but the total sum of all the variables and values that must not exceed 4096 bytes, so breaking one variable value into multiple variables will end with the same problem.
Appreciate the insight and effort here! We have this under advisement and will consider this further but for now I am, respectfully, going to close this PR out.
