gjcarneiro / yacron

A modern Cron replacement that is Docker-friendly
MIT License
449 stars 38 forks source link

Shell Reporter #50

Closed hhergeth closed 3 years ago

hhergeth commented 3 years ago

Description

I added the shell reporter as a new reporter implementation which simply executes a shell command on failure or success.

The motivation was to be able to send notifications to Gotify via a curl call. The implementation uses the same code that is used to execute the command of jobs. Additionally a small set of environment variables is added to the report command containing information about the state of the job.

Example

jobs:
  - name: test-01
    command: echo "foobar" && exit 123
    shell: /bin/bash
    schedule: "* * * * *"
    onFailure:
      report:
        shell:
          command: echo "Error code $retcode"

The user can specify the command the same way that is used for the job itself. In the same spirit the shell can be chosen as well.

Limitations

At the moment no unit test is provided. If required I can add one although I am unsure of how to properly test this (best idea so far is to write content to a file in the shell command being executed by the reporter). Furthermore I haven't updated the documentation. Will do that in case the feature is generally liked.

gjcarneiro commented 3 years ago

I think it's a great idea. We need as escape hatch like that.

The only thing I don't like so much is the naming of the env. vars, I would suggest uppercase and prefixed by YACRON_:

Wrt to the code, it looks fine to me, the only thing to fix is that whenever you compare with None (xxx != None) you should use is not instead of !=, i.e. xxx is not None.

For the test, having a simple command that writes something to a file in a temporary dir should be fine.

hhergeth commented 3 years ago

Yeah those env variable names are much better!

I changed all the things you mentioned; added a unit test and updated the docs. Let me know if there is anything else that you would like me to change.

gjcarneiro commented 3 years ago

Thank you for the PR :+1: