Make max. upload size configurable via env variable
Closed this issue · 6 comments
The current upload restriction of 2 MB is pretty low. I suggest to introduce another environment variable to configure the max. upload filesize.
I know that we can use the workaround shown in #17. But this is such an essential setting that you should not have to create a custom config file to change it. Users of this image don't want to mess around with PHP settings - all they want is a quick admin panel for their database.
I had a short discussion with @tianon about this in IRC. Instead of making it configurable I opted for a sane higher limit by default.
The issue here is not „I need to easily change the values across my fleet“, the issue is „I want to upload large files“. Adding a redundant configuration knob via an environment variable makes both the image itself as well as setting up the container more complex. Adding this bit to the README increases the noise for next to no benefit.
The updated values in #37 should allow for any use cases that are reasonably handled by PHP, while still preventing stupid mistakes. If you need to import your 5 gigabyte SQL dump you are free to use the well known php.ini path to increase the values or you should simply use the MySQL CLI interface.
I have a similar issue with max_input_vars. I have a mildly complex table with ~150 columns and try to alter the table blows up due to the limit. I can build a custom image but something bumped up would be nice. Or a generic way to pass php ini values into the container would be great as well.
@travisghansen Makes sense. Re-opening. Feel free to propose a PR with a limit that suits your needs, otherwise I'll do when I get around to it.
Regarding the generic way to override: You are able to set specific ini settings on the command line using -d. You will need to copy the while CMD I configured within the image, though. I believe that that + the ability to build an own image / bind mount a file is sufficient and we don't need an adminer specific methodology.
@TimWolla yeah understood. I can think of several ways to approach it. We could do PHP_ARGS env variable which could be nice. Downside is that's pretty specific to running behind php -S which isn't great in an environment with any kind of traffic (ie: handle more than 1 request at a time).
We could introduce a PHP_INI env variable that just gets written to disk on startup with I think is more portable should you ever decide to run behind nginx or the like.
If I put together an MR do you have a preferred approach? Any of the above or something different altogether?
If I put together an MR do you have a preferred approach?
My preferred approach would be: Hardcode some sane limit, like I did in #37. The default for max_input_vars is 1000. I guess something like 25000 should be good (which allows for 250 columns with 10 fields each, I count 8 fields per column at a quick glance in “Alter Table”).
Anything else can be done by a child image or by bind mounting an .ini file to /usr/local/etc/php/conf.d/whatever.ini at runtime (-v /etc/adminer.ini:/usr/local/etc/php/conf.d/my-adminer-changes.ini). No need to set up a special facility for Adminer. I don't expect anyone to update .ini settings for a tool that is intended for development, apart from the resource limits (and we are providing sane values for them out of the box).
For posterity, even though @TimWolla expressed his concern that adding an environment variable will make Docker image more "complex", while somewhat true, people should be able to decide if they're going to make stupid decisions for themselves, not being stopped from doing so by anyone with an opinion out there.
That being said, environment variables aren't necessary as entrypoint command can be overriden with the following:
command: [ "php", "-d", "upload_max_filesize = 10G", "-d", "post_max_size = 10G", "-S", "[::]:8080", "-t", "/var/www/html" ]
php -d allows overriding php.ini flags on process start so no need to beg for environment variables no more.
Tried in Kubernetes cluster just now, worked fine.
