Closes issue #153 #157
Closes issue #153 #157
Conversation
| @@ -86,6 +86,7 @@ function startContainer() { | |||
|
|
|||
| docker exec -it $CONTAINER_NAME /bin/bash -c "echo NGINX_PORT=$nginxPort > $BENCHMARK_KIT_PATH/.env.local" | |||
| docker exec -it $CONTAINER_NAME /bin/bash -c "echo HOST_SOURCE_CODE_PATH=$hostSourceCodePath >> $BENCHMARK_KIT_PATH/.env.local" | |||
| docker exec -it $CONTAINER_NAME /bin/bash -c "chown -R phpbenchmarks:phpbenchmarks /var/www/benchmark" | |||
There was a problem hiding this comment.
I think this approach is about temporary fix.
If fixing permission issue persistently, it should modify docker/Dockerfile.
But I think it's not a great idea because it should release new Docker image version on Docker hub.
There was a problem hiding this comment.
Releasing a new version on Duckerhub is not a problem, for now 4.0.0 is in BETA and could be modified if needed.
I'm not sure this is a good idea, because changing permissions on host could be dangerous: if path to source code is / because of a bug or something else, permissions for ALL files on your hosts are modified.
I'm thinking about a warning to tell the user to change permissions manually, with an example, but no doing it automatically.
There was a problem hiding this comment.
We can use Bash if syntax to check the permission and print warning message on this script and remind developers should use this command to change permission by themselves.
Do you have any suggested changes about above my suggestions?
There was a problem hiding this comment.
maybe we can try to do the thing, and on error, show the warning? it's easiest and will always work as intendend.
There was a problem hiding this comment.
Could you have the suggested changes?
Changed log
/var/www/benchmarkfolder.README.