Fix script copying

[ptersilie]

I'm not very familiar with Docker so I'm not sure what this does?

Also wouldn't it be safer to just copy config.template instead of moving it, so people don't accidentally push their changes?

[syamgk]

this copies files to container. and inside the container it is running mv command to rename config.template

[ptersilie]

Ah that makes sense. Can people edit the config.py file after it has been copied to the container or do they need this first? In the latter case I might add some explanation to the readme.

[syamgk]

as of now yes we need to edit the config.template first and then build the image, But this is not a good container pattern as the people will be consuming the container from the registry at the end. we need to put a little work on that part For now we can go with this and later we can clear that part

[ptersilie]

I think I would prefer users to copy the config.template to config.py first and edit that and then build the image, as this is closer to what we describe in the ReadMe.

But then again I'm no Docker expert. @Psidium You added the initial Docker stuff. Any opinions on this?

[ptersilie]

Ah I think I understand now. docker build doesn't automatically copy files into the image itself, but references the files from within the github folder. So if I then change those files, the docker image uses the changed files.

So what you were saying earlier is that it's not good practice to have the docker image reference files that have not been copied into the docker image itself? However, isn't it easier this way to later on adjust the config.py without having to rebuild the docker image each time?

[syamgk]

Here we are hard-coding the urls, channel names and all at the time of build. which is a bad practice

So we should find a way to build containers, and later while running either pass some env variables or should find a mechanism to load these values dynamically to the running container.

[ptersilie]

I just tested this. If I run the docker image and then edit the config.py the flask service automatically reloads the config and restarts the service. So even while running the docker image you can just change the config.py from within the github checkout and the service automatically picks up the changes. Which is why I think copying the config.py into the Docker image is not very convenient as we then have to rebuild and rerun the docker image every time we change something in config.py.

In other words: I think the current version already does what you want it to do.

[Psidium]

Hey!

I'm with you, @ptersilie, to actually follow the readme the users need to create their config.py themselfs (while following the readme and adding in their configs).

This change don't add anything, only adds a new step in the build which is already taken care when the user runs the docker image with the option -v "$(pwd)":/home/app

[syamgk]

but what i found is that, it is not a good practice to edit something inside a running container by execing into the container and accessing the files. the better way is to pass environment variables and a docker-entrypoint script to modify config according to the env. or a web interface to edit the config files is preferable.

[ptersilie]

But we don't need to exec into the container to edit the files. If I understand everything correctly, then currently the config.py is outside of the container and will be referenced from within the container. So you can just edit the config.py in the mattermost-github-integration folder and the docker container will pickup the changes from there. No need to edit anything within the container itself.