Add matrix notification module #45823

jcgruenhage · 2018-09-18T22:52:08Z

SUMMARY

This is a matrix notification module, to live alongside telegram, slack and the rest of the crowd.

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

matrix

ANSIBLE VERSION

ansible 2.8.0.dev0 (matrix-notification-module bd6dec4406) last updated 2018/09/19 00:48:30 (GMT +200)
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/jcgruenhage/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/jcgruenhage/dev/ansible/lib/ansible
  executable location = /home/jcgruenhage/dev/ansible/bin/ansible
  python version = 3.6.6 (default, Jul 19 2018, 14:25:17) [GCC 8.1.1 20180712 (Red Hat 8.1.1-5)]

ansibot · 2018-09-18T23:04:30Z

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:114:0: trailing-whitespace Trailing whitespace

The test ansible-test sanity --test boilerplate [explain] failed with 2 errors:

lib/ansible/modules/notification/matrix.py:0:0: missing: __metaclass__ = type
lib/ansible/modules/notification/matrix.py:0:0: missing: from __future__ import (absolute_import, division, print_function)

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:49:0: SyntaxError: Non-ASCII character '\xc3' in file /root/ansible/lib/ansible/modules/notification/matrix.py on line 50, but no encoding declared; see http://www.python.org/peps/pep-0263.html for details

The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:49:0: SyntaxError: Non-ASCII character '\xc3' in file /root/ansible/lib/ansible/modules/notification/matrix.py on line 50, but no encoding declared; see http://python.org/dev/peps/pep-0263/ for details

The test ansible-test sanity --test import --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:75:0: ImportError: No module named 'matrix_client'

The test ansible-test sanity --test import --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:75:0: ModuleNotFoundError: No module named 'matrix_client'

The test ansible-test sanity --test import --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:75:0: ModuleNotFoundError: No module named 'matrix_client'

The test ansible-test sanity --test pep8 [explain] failed with 7 errors:

lib/ansible/modules/notification/matrix.py:46:1: W293 blank line contains whitespace
lib/ansible/modules/notification/matrix.py:60:1: W293 blank line contains whitespace
lib/ansible/modules/notification/matrix.py:68:43: W291 trailing whitespace
lib/ansible/modules/notification/matrix.py:77:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/notification/matrix.py:114:1: W293 blank line contains whitespace
lib/ansible/modules/notification/matrix.py:117:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/notification/matrix.py:120:1: E305 expected 2 blank lines after class or function definition, found 1

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/notification/matrix.py:0:0: E105 GPLv3 license header not found in the first 20 lines of the module
lib/ansible/modules/notification/matrix.py:0:0: E307 version_added should be 2.8. Currently 2.7
lib/ansible/modules/notification/matrix.py:0:0: E321 Exception attempting to import module for argument_spec introspection, 'No module named 'matrix_client''

click here for bot help

jcgruenhage · 2018-09-19T06:32:42Z

Looks like I've still got some work to do :D

ansibot · 2018-09-19T17:46:40Z

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test docs-build [explain] failed with the error:

Command "/usr/bin/python test/sanity/code-smell/docs-build.py" returned exit status 1.
>>> Standard Error
Traceback (most recent call last):
  File "test/sanity/code-smell/docs-build.py", line 99, in <module>
    main()
  File "test/sanity/code-smell/docs-build.py", line 17, in main
    raise subprocess.CalledProcessError(sphinx.returncode, cmd, output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['make', 'singlehtmldocs']' returned non-zero exit status 2.

The test ansible-test sanity --test boilerplate [explain] failed with 2 errors:

lib/ansible/modules/notification/matrix.py:0:0: missing: __metaclass__ = type
lib/ansible/modules/notification/matrix.py:0:0: missing: from __future__ import (absolute_import, division, print_function)

click here for bot help

jborean93 · 2018-09-20T01:38:19Z

@jcgruenhage it is a lot easier and quicker to run the sanity tests locally. This saves you waiting for Shippable to run and you can run individual sanity tests instead of the whole bunch. To do this you can run

ansible-test sanity --docker default --test <test here>

You can double up on tests by appending more --test <another test> --test <one more test> to the command. For example if you wanted to run ansible-doc, docs-build and boilerplate locally you can do

ansible-test sanity --docker default --test ansible-doc --test docs-build --test boilerplate

jcgruenhage · 2018-09-20T05:37:02Z

Oh cool, thanks 👍

jcgruenhage · 2018-09-20T07:30:15Z

@jborean93 In what directory are these supposed to be run? If run in the repository root I get a bunch of things for packages in the venv and in the directory of the module it fails to run..

jborean93 · 2018-09-20T19:57:32Z

@jcgruenhage it's best to run source hacking/env-setup in your Ansible git checkout to ensure you are using the Ansible test for the current code. If you are getting errors still, can you share the output and we can help.

jcgruenhage · 2018-09-21T09:22:52Z

I did that. In the root of the repository I get a bunch of these:

ERROR: venv/lib/python2.7/site-packages/yaml/emitter.py:0:0: missing: __metaclass__ = type
ERROR: venv/lib/python2.7/site-packages/yaml/emitter.py:0:0: missing: from __future__ import (absolute_import, division, print_function)

They clearly don't matter, because it's venv packages and not what I am contributing.

If I run it in the directory of the module I'm contributing, I get this:

 jcgruenhage  (e) venv  …  ansible  modules  notification  ansible-test sanity --docker default --test boilerplate
Sending build context to Docker daemon  189.4kB
Step 1/8 : FROM debian:7.7
 ---> 6d129b37ec3b
Step 2/8 : MAINTAINER Rob McQueen
 ---> Using cache
 ---> 4de027bf81db
Step 3/8 : RUN apt-get update && apt-get install -y   python-dev   python-virtualenv   sudo
 ---> Using cache
 ---> b9e3a3bd6be6
Step 4/8 : RUN mkdir /opt/ansible && virtualenv /opt/ansible/venv &&   /opt/ansible/venv/bin/pip install ansible
 ---> Running in bcdac859859e
New python executable in /opt/ansible/venv/bin/python
Installing distribute............................................................................................................
.................................................................................done.
Installing pip...............done.
Downloading/unpacking ansible
  Cannot fetch index base URL http://pypi.python.org/simple/
  Could not find any downloads that satisfy the requirement ansible
No distributions at all found for ansible
Storing complete log in /root/.pip/pip.log
The command '/bin/sh -c mkdir /opt/ansible && virtualenv /opt/ansible/venv &&   /opt/ansible/venv/bin/pip install ansible' returned a non-zero code: 1
Traceback (most recent call last):
  File "/usr/bin/ansible-test", line 69, in <module>
    main(context)
  File "/usr/bin/ansible-test", line 45, in main
    '-t', DOCKER_CONTAINER, '.']
  File "/usr/lib64/python2.7/subprocess.py", line 190, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['docker', 'build', '-t', 'testbox', '.']' returned non-zero exit status 1

ansibot · 2018-09-29T12:11:08Z

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:74:0: misplaced-future __future__ import is not the first non docstring statement

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:74:0: SyntaxError: from __future__ imports must occur at the beginning of the file

The test ansible-test sanity --test import --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:74:0: SyntaxError: from __future__ imports must occur at the beginning of the file

The test ansible-test sanity --test import --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:74:0: SyntaxError: from __future__ imports must occur at the beginning of the file

The test ansible-test sanity --test import --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:74:0: SyntaxError: from __future__ imports must occur at the beginning of the file

The test ansible-test sanity --test import --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:74:0: SyntaxError: from __future__ imports must occur at the beginning of the file

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/modules/notification/matrix.py:48:7: W291 trailing whitespace
lib/ansible/modules/notification/matrix.py:130:5: E265 block comment should start with '# '

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:0:0: E321 Exception attempting to import module for argument_spec introspection, 'from __future__ imports must occur at the beginning of the file (matrix.py, line 74)'

click here for bot help

ansibot · 2018-09-29T16:36:17Z

@Deepakkothandan @Jmainguy @bjolivot @bkimble @drew-russell @fabulops @garbled1 @jcftang @jpmens @makaimc @mcodd @mpdehaan @pb8226 @shirou @tksmd @tonyseek @tyouxa @weaselkeeper @willybarro @zimbatm

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

jcgruenhage · 2018-10-17T08:21:00Z

It's been a while, is there anything more I can do aside from sending a "bump" comment every now and then?

Jmainguy · 2018-10-17T13:38:30Z

I have found asking for a review in #ansible-devel on the Freenode IRC has been helpful in moving things along. I don't have a matrix account or time to test this right now, but someone else in that channel likely will.

jcgruenhage · 2018-10-17T18:05:12Z

I'll try that, thanks

bcoca · 2018-10-17T19:31:42Z

lib/ansible/modules/notification/matrix.py

+    msg_html:
+        description:
+            - HTML form of the message to send to matrix
+        required: true


i would have single 'msg' and then a 'type' option, this follows the pattern of other modules.

That would be fit if the message sent contains either a plain text or an html message. Matrix sends both, a plain text variant (usually markdown) and also an HTML variant in the content of a text message event, unless there is no formatting (then the HTML variant is omitted). Because of that, the two different args are required as is.

bcoca · 2018-10-17T19:32:43Z

lib/ansible/modules/notification/matrix.py

+        description:
+            - The password to log in with
+notes:
+    - Requires matrix-client on the executing host.


add 'python library' , then the 'pip install' becomes redundant, some people cannot use pip to install libraries

This was adapted from pushbullet.py, but you're concern is definitely valid, will change it.

Thanks for this PR. Few minor things that are easier for me to just fix, than explain and get you to fix. * We suggest using `- name:` for examples, as Ansible best practice is to name your tasks * To prevent secrets being leaked out use `no_log` in argspec * use `requirements:` in `DOCUMENTATION`

ansibot · 2018-10-23T11:29:38Z

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module matrix" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible/lib/ansible/modules/notification/matrix.py, line 3, column 1,
found a duplicate dict key (requirements). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module matrix" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible/lib/ansible/modules/notification/matrix.py, line 3, column 1,
found a duplicate dict key (requirements). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module matrix" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible/lib/ansible/modules/notification/matrix.py, line 3, column 1,
found a duplicate dict key (requirements). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module matrix" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible/lib/ansible/modules/notification/matrix.py, line 3, column 1,
found a duplicate dict key (requirements). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module matrix" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible/lib/ansible/modules/notification/matrix.py, line 3, column 1,
found a duplicate dict key (requirements). Using last defined value only.

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

lib/ansible/modules/notification/matrix.py:51:1: key-duplicates DOCUMENTATION: duplication of key "requirements" in mapping

click here for bot help

ansibot · 2018-10-23T12:19:07Z

The test ansible-test sanity --test validate-modules [explain] failed with 4 errors:

lib/ansible/modules/notification/matrix.py:0:0: E332 AnsibleModule.mutually_exclusive.0: expected list @ data['mutually_exclusive'][0]. Got 'password'
lib/ansible/modules/notification/matrix.py:0:0: E332 AnsibleModule.mutually_exclusive.1: expected list @ data['mutually_exclusive'][1]. Got 'token'
lib/ansible/modules/notification/matrix.py:0:0: E332 AnsibleModule.required_together.0: expected list @ data['required_together'][0]. Got 'user_id'
lib/ansible/modules/notification/matrix.py:0:0: E332 AnsibleModule.required_together.1: expected list @ data['required_together'][1]. Got 'password'

click here for bot help

gundalow

Looks great, thank you for the prompt fixes

gundalow · 2018-10-24T07:55:49Z

Thank you for the new module. It's been merged into devel, and will be released in Ansible 2.8.

* Add matrix notification module * try to make ansibot happy * docs * fix typo in encoding * is ansibot happy now? * change matrix python lib requirement description * Example formatting & no_log Thanks for this PR. Few minor things that are easier for me to just fix, than explain and get you to fix. * We suggest using `- name:` for examples, as Ansible best practice is to name your tasks * To prevent secrets being leaked out use `no_log` in argspec * use `requirements:` in `DOCUMENTATION` * Clean up argument requirements * Remove requirements duplicate * not sure on syntax with these, were adapted from an example elsewhere

Add matrix notification module

bd6dec4

try to make ansibot happy

b04fbff

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Sep 19, 2018

mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Sep 19, 2018

jborean93 removed the needs_triage Needs a first human triage before being processed. label Sep 20, 2018

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 29, 2018

jcgruenhage added 2 commits September 29, 2018 13:54

docs

a6ce0e1

fix typo in encoding

9842e75

ansibot removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Sep 29, 2018

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Sep 29, 2018

is ansibot happy now?

4efb8e4

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Sep 29, 2018

ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Sep 29, 2018

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 7, 2018

bcoca requested changes Oct 17, 2018

View reviewed changes

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Oct 17, 2018

change matrix python lib requirement description

e94bd8e

ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 17, 2018

jcgruenhage added 2 commits October 23, 2018 13:59

Clean up argument requirements

b55968e

Remove requirements duplicate

108407a

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Oct 23, 2018

not sure on syntax with these, were adapted from an example elsewhere

08d0791

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Oct 23, 2018

gundalow approved these changes Oct 24, 2018

View reviewed changes

gundalow merged commit 01bee0c into ansible:devel Oct 24, 2018

ansible locked and limited conversation to collaborators Jul 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add matrix notification module #45823

Add matrix notification module #45823

jcgruenhage commented Sep 18, 2018

ansibot commented Sep 18, 2018

jcgruenhage commented Sep 19, 2018

ansibot commented Sep 19, 2018

jborean93 commented Sep 20, 2018 •

edited

jcgruenhage commented Sep 20, 2018

jcgruenhage commented Sep 20, 2018

jborean93 commented Sep 20, 2018

jcgruenhage commented Sep 21, 2018

ansibot commented Sep 29, 2018

ansibot commented Sep 29, 2018

jcgruenhage commented Oct 17, 2018

Jmainguy commented Oct 17, 2018

jcgruenhage commented Oct 17, 2018

bcoca Oct 17, 2018

jcgruenhage Oct 17, 2018

bcoca Oct 17, 2018

jcgruenhage Oct 17, 2018

ansibot commented Oct 23, 2018

ansibot commented Oct 23, 2018

gundalow left a comment

gundalow commented Oct 24, 2018

Add matrix notification module #45823

Add matrix notification module #45823

Conversation

jcgruenhage commented Sep 18, 2018

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ansibot commented Sep 18, 2018

jcgruenhage commented Sep 19, 2018

ansibot commented Sep 19, 2018

jborean93 commented Sep 20, 2018 • edited

jcgruenhage commented Sep 20, 2018

jcgruenhage commented Sep 20, 2018

jborean93 commented Sep 20, 2018

jcgruenhage commented Sep 21, 2018

ansibot commented Sep 29, 2018

ansibot commented Sep 29, 2018

jcgruenhage commented Oct 17, 2018

Jmainguy commented Oct 17, 2018

jcgruenhage commented Oct 17, 2018

bcoca Oct 17, 2018

Choose a reason for hiding this comment

jcgruenhage Oct 17, 2018

Choose a reason for hiding this comment

bcoca Oct 17, 2018

Choose a reason for hiding this comment

jcgruenhage Oct 17, 2018

Choose a reason for hiding this comment

ansibot commented Oct 23, 2018

ansibot commented Oct 23, 2018

gundalow left a comment

Choose a reason for hiding this comment

gundalow commented Oct 24, 2018

jborean93 commented Sep 20, 2018 •

edited