Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for a Pushover alerter #136

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

kylemart
Copy link

@kylemart kylemart commented Jan 2, 2021

Adding support for notifications via https://pushover.net/.

@EricJMarti
Copy link
Owner

docker_run.bash's command line arguments are getting pretty complicated.. how do you feel about requiring users to configure this type of alerter via a YAML alerter config?

@kylemart
Copy link
Author

kylemart commented Feb 6, 2021

docker_run.bash's command line arguments are getting pretty complicated.. how do you feel about requiring users to configure this type of alerter via a YAML alerter config?

I think it would make sense to restrict alerter configuration to a YAML config 🤔The YAML alerter config section could be made generic, something like:

alerters:
  - name: pushover
    configure:
      api_token: APITOKEN
      user_key: USERKEY
  - name: discord
    configure:
      ...

(Side-thought: You could also auto-generate documentation by dynamically loading the concrete alerters, and creating a yaml template based off the required kwargs like I depicted in the above code block.)

And of course, when it comes to loading alerters, loading an instance could be something like:

alerter = AlerterFactory.get_class(config['name']).initialize(config['configure'])

@mgt20
Copy link

mgt20 commented Apr 20, 2021

I would find use out of this too. I was able to replicate the changes from kylemart:kylemart/pushover-support into a local copy (have yet to create a fork) I made of EricJMarti:main while avoiding conflicts (such as adding new args getopts to docker_run.bash for user_key and api_token, I used "u" and "y", respectively).

I was not able to get this to run with either inline arguments or using the alerters.yaml file. In both cases, I'm getting an:
Exception: the "pushover" alerter type does not exist in the registry

Is there something obvious I'm missing? Would love to help implement this feature if possible (though I'm a bit elementary in Python and YAML).

Happy to create a fork and point to the code I have for easier debugging. Though, I mainly implemented @kylemart's existing work here and just changed some things around to avoid conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants