tdmalone / slackemon

Inspired by Pokémon Go, now you can catch and battle Pokémon with your teammates on Slack!
GNU General Public License v3.0
10 stars 3 forks source link

Adds cron support to Docker installation #20

Closed Naramsim closed 7 years ago

Naramsim commented 7 years ago

Hi, this PR enables cron to run every minute, invoking the following command

php cron.php --token=$SLACKEMON_CRON_TOKEN

Is it correct to run it every minute? (* * * * *) Or this should be user-customizable? Right now it is hard-coded

tdmalone commented 7 years ago

It is correct to run every minute at the moment, because both the spawn chance and the battle updates expect a chance every minute to do their work (probably should make this more robust in future though).

How would I test this? I tried checking this out locally and re-building an image, but cron doesn't seem to be running when my container is running (I've stuck an error_log() call towards the top of the cron script which is successfully writing to the log when I invoke it manually at http://localhost/cron.php, but it doesn't seem to be happening automatically every minute). I'm not sure if I'm just not using Docker properly, or if it's actually not working. :)

Naramsim commented 7 years ago

Hi, while the container is running, can you docker exec -it slackemonInstance bash and check if the cron has been started (ps -aux) and what's inside the cron scripts(crontab)?

tdmalone commented 7 years ago

Okkk... turns out cron couldn't find php ;) Guided by this answer I found CRON was exiting with error code 127 which means it can't find the command.

So, it runs correctly with /usr/local/bin/php /var/www/html/cron.php. I've edited the Dockerfile and am just rebuilding the image now to test that it works from the start (side note: is there a quicker way to 're-run' your Dockerfile without having to rebuild the entire image??)

One other thing we'll need to do is look at the generation of the token for this, as at this stage of the game the cron token doesn't exist as an env var yet. Or.... maybe we rethink this, and don't even require the token over the command line, because, of course it's authorised on the command line. The token could just be required if the script is called over the web.

tdmalone commented 7 years ago

Ok, done, and token is no longer required when run over cli: https://github.com/tdmalone/slackemon/commit/4ee1307ea8428003d8dc21bec29482a4a6aadab6