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 Discord transport #8748

Merged
merged 17 commits into from Jun 13, 2018

Conversation

Projects
None yet
4 participants
@theherodied
Contributor

theherodied commented May 21, 2018

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

@laf

This comment has been minimized.

Member

laf commented May 21, 2018

Needs the webui + docs doing as well :p Thanks for adding this though

theherodied added some commits May 21, 2018

@theherodied

This comment has been minimized.

Contributor

theherodied commented May 21, 2018

From sdef2

Discord server returns something like "empty response" http code Instead of http status 200
Don't remember which code exactly, but you need to modify the script accordingly else librenms thinks the alert failed
Line 55

@theherodied

This comment has been minimized.

Contributor

theherodied commented May 23, 2018

Returns a 204 empty response on success

theherodied added some commits May 23, 2018

@theherodied theherodied changed the title from Discord transport 001 to Add Discord transport May 23, 2018

@theherodied

This comment has been minimized.

Contributor

theherodied commented May 23, 2018

@laf not sure what to do about the sql. I'm not entirely sure what's needed but tried it anyway. Even with the new sql it's giving me "Missing config name or value"

@f0o

knit picking

/**
* Discord.php
*
* LibreNMS storage discovery module for Nimble Storage

This comment has been minimized.

@f0o

f0o May 23, 2018

Member

wat?

This comment has been minimized.

@theherodied

theherodied May 23, 2018

Contributor

Copy and Paste error.

theherodied added some commits May 23, 2018

@laf

This comment has been minimized.

Member

laf commented May 24, 2018

More code needed in html/includes/forms/config-item.inc.php.

Also, the changes to html/pages/settings/alerting.inc.php have muddled up. You've got slack/discord and rocket code mixed up.

theherodied and others added some commits May 25, 2018

@laf

This comment has been minimized.

Member

laf commented May 27, 2018

@theherodied I've fixed the issue with the html page in the settings.

Tested and works well.

Just needs docs doing now.

@theherodied

This comment has been minimized.

Contributor

theherodied commented May 29, 2018

@laf, I can't seem to get it working on mine. Not sure what's up with it.
I added the webhook url and hit test but nothing shows up. The "Test transport" button turns red then back to blue.

@theherodied

This comment has been minimized.

Contributor

theherodied commented May 29, 2018

@laf , got it working. Just the "Test transport" button doesn't work.

Only thing I've gotten working so far is the basic webhook url. Not sure if I can get the channel, username, etc... working or if it's needed. Webhook is per channel so that might not matter.

@laf

This comment has been minimized.

Member

laf commented May 31, 2018

Only thing I've gotten working so far is the basic webhook url. Not sure if I can get the channel, username, etc... working or if it's needed. Webhook is per channel so that might not matter.

Do you mean message a specific user? I've not looked at the Discord api docs but channel works ok so I'm happy with that. If you can sort the docs we can get this merged in

@laf laf added this to the 1.41 milestone May 31, 2018

@theherodied

This comment has been minimized.

Contributor

theherodied commented May 31, 2018

@theherodied

This comment has been minimized.

Contributor

theherodied commented May 31, 2018

@laf I'm good with this for now as well. It has been working well the past few days I've tested it.

Added docs and alphabetized the list.

No longer relevant

@laf laf changed the title from Add Discord transport to WIP Add Discord transport Jun 13, 2018

@laf laf changed the title from WIP Add Discord transport to Add Discord transport Jun 13, 2018

@laf

laf approved these changes Jun 13, 2018

Tested and works fine. Thanks @theherodied :)

@laf laf removed the Feature label Jun 13, 2018

@laf laf merged commit a6c3289 into librenms:master Jun 13, 2018

2 of 3 checks passed

WIP work in progress
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

mattie47 added a commit to mattie47/librenms that referenced this pull request Jul 2, 2018

Added Discord transport (librenms#8748)
DO NOT DELETE THIS TEXT

#### Please note

> Please read this information carefully. You can run `./scripts/pre-commit.php` to check your code before submitting.

- [x] Have you followed our [code guidelines?](http://docs.librenms.org/Developing/Code-Guidelines/)

#### Testers

If you would like to test this pull request then please run: `./scripts/github-apply <pr_id>`, i.e `./scripts/github-apply 5926`

@lock lock bot locked as resolved and limited conversation to collaborators Aug 12, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.