Docker support

[Taxel]

As you might have seen, I have pushed the work in progress docker branch. I am currently testing the built image in my home setup (with this image as well as Plex running in Kubernetes administered via Rancher) but I have made some changes to how the script works, files have been reorganized etc. so I wanted to discuss them here first before even submitting a pull request:

• I rebased the branch on my changes in Update to use guids from python-plexapi to fix Plexmovieagent breakage #87, if we wait for the official python-plexapi package to get updated this will have to be redone
• I changed the file structure: With configuration, cache, logs, and source code in one directory the project felt unorganized, so I should have probably done this before even making the project open source.
• The docker image is based on @twolaw 's Dockerfile. The main differences are, that I am using a multistep build (unnecessarily I might add, this does not really make the resulting image much smaller, but it was fun to play around with) and that the script does not run once by default but on a cronjob, so the container can be configured and then left running forever. One can, however, pass the environment variable "RUN_ONCE" with a non-empty value and the script will only be run once, after which the container shuts down. Changing the cron schedule is also done via an environment var: CRON_SCHEDULE
• Running this in Docker also means that via volume mounts and multiple containers, one can use different configs for multiple users and/or Plex servers, solving Allow custom envfile path to be used #70 indirectly (only in Docker, of course. Not when running this with Python. Since I extracted all the filepaths to config.py this should be trivial to implement, though)

What might be the most controversial change, is that I am rerouting the log outputs to stdout if the terminal is non-interactive. I use it to detect if the script is running in a container since Docker containers should output to stdout/stderr and let Docker take care of the logging. Do you agree the script should no longer write to lastRun.log if it is running in a container and is the sys.isatty(0) check okay or should I add an argument like python3 main.py --log-to-stdout so this is easier to control?

Since the file structure changes alone constitute some manual work when upgrading from an older version of PlexTraktSync, I also propose we create a release 0.9.0 before merging the pull request for this once the feature is complete and then release 1.0.0 along with the docker image (I'd like to try Github Packages as the image registry and create a Github action that automatically builds, tags and pushes each new release in the future)

@glensc @twolaw what do you think about these changes? If you want to test them out locally it should suffice to docker-compose up --build after pulling the branch. Are there other things I should add to the image prior to release (for example atm the container always runs as root. I didn't deem it necessary to make it possible to change this since the container just mounts the config directory and is not accessible from the outside because it does not listen for incoming connections)

Greez
Taxel

[glensc]

You should create PR anyway, even if it does not get merged (or why be afraid of PR in the first place?), otherwise can't see or comment on the code. if I comment in the commit, then the comments are spread around and no overview of feedback.

[Taxel]

I mainly did not submit a PR earlier because this is not a completed feature and there are some things that should be discussed before even considering merging this (the points I mentioned above). Also, after watching the first couple of minutes of GitHub Universe, I realized this should not be an issue but a discussion, a feature I didn't know GitHub has. To try this out, I am converting this to a discussion right after writing this.
EDIT: the PR can be found here: #89

[simonc56]

Good move. I like it. I will test it later.
Version number helps a lot for support. 👍

I think presence of all configs files should be checked before launching crond.
If the user forget to run interactively at first run (to setup plex/trakt credentials), he will only see the problem in logs later when script will be triggered (2 hours later or less), right ?

[simonc56]

An option to choose log to file is good.

[Taxel]

Good catch on checking the presence of logfiles before starting the cronjob. I'll add this to the shell script.

If I understand you correctly, you mean the --log-to-stdout arg, that is always passed in entry.sh or do you mean when running this in Docker the user should be able to pass an env var that enables log to file?

[simonc56]

Two differents subjects here :

1. I think entry.sh should check the presence of all needed config files (especially .env and .pytrakt.json) before running the crond. If a file is missing, warn user and don't run crond. Even better : directly run the setup process to populate those files, then run crond.

2. I answer your question Do you agree the script should no longer write to lastRun.log if it is running in a container and my answer is : if the user can change it (with an argument in commandline) that's ok for me. My fault, I misunderstood the question.

[Taxel]

1. since a Docker container always runs in a non-interactive terminal by default, the only option I see would be to check for the existence of .env and .pytrakt.json and print out a warning, but leave the container running (the warning can be printed in entry.sh on startup). If the container is not left running, it is nontrivial to get the script configured in a Kubernetes/Rancher setup afaik (since you somehow have to start the container with docker run in an interactive shell, with the same volume mounts etc.), whereas you can just exec (or use the builtin Rancher shell) into a running container to execute the setup.

2. I meant to ask if it would be okay if the script will always run in --log-to-stdout mode when it is inside a docker container. I think that's how Docker containers should log, so I would add the argument parsing to main.py but always call the script with this argument from entry.sh, since this gives users the freedom to log into a file if they are running it directly (via Python or cronjob) but keeps it as simple as possible for Docker users, since passing the environment variable that enables logging to a file would then require an additional volume mount, making the container usage instructions more complex while not giving benefits for any scenario I can imagine (but maybe there is one I just can't think of).

[simonc56]

1. yes a warning printed out by entry.sh is enough.

2. i agree.

[glensc]

I've never used cron, because it seemed to me it would record watch time earlier, while I was still watching movie. something like:

1. cron was set to invoke at */30
2. movie ended at *:45
3. but the job ran at *:30 and it was recorded to track as *:30

maybe there's something more to it, but I just disabled cron, and continue to invoke sync script manually when I'm not watching anything at the time.

[simonc56]

As far as I know, Plex marks a movie as watched when you played at least 9x% of the movie.
So for a 3h+ movie the watched status can be switched before the actual end of the movie.

[glensc]

Yeah, I guessed it's something like that, so I prefer that job is run only when I'm not watching to prevent premature submission.

For example, I hit pause at the end of 10m, I don't consider the movie watched by myself yet.

[simonc56]

I worked on a PR to improve connection to Plex.
It adds multiple steps if connection fails : update plex url if needed, retry without SSL, retry with a localhost ip.
Do I wait for 0.9 version release with the new file structure ?

[glensc]

Most up to date discussion is in issue instead:

• Feature request: official docker repository #411