search

mqtt-tools / mqttwarn

A highly configurable MQTT message router, where the routing targets are notification plugins, primarily written in Python.
https://mqttwarn.readthedocs.io/
Eclipse Public License 2.0
955 stars 183 forks source link

Python 3 support #336

Open jpmens opened 6 years ago

jpmens commented 6 years ago

Karl rightly asks whether we intend to support Python 3. The only valid response to the question is "yes".

amotl commented 6 years ago

Thanks for asking, @jpmens and Karl. I would like to extend that answer to "definitively yes" ;]. Support for Python3

dlangille commented 4 years ago

Python 2.7 is EOL in about 6 weeks. Any news?

amotl commented 4 years ago

Dear Dan,

thanks for asking.

While [1] has been left behind due to my failure to continue working on that appropriately (sorry @jpmens!), I am still willing to catch up on that and finally update the codebase to support Python 3.

Comparing the branches [2] shows it should be doable without too much hassle as I believe most of the improvements landed in the plugin area. The test harness added through 257d0b83 will definitively help here.

With kind regards, Andreas.

[1] https://github.com/jpmens/mqttwarn/tree/develop [2] https://github.com/jpmens/mqttwarn/compare/develop...master

amotl commented 4 years ago

Comparing the branches [2] shows bringing "develop" up to speed with "master" should be doable.

I've just quickly scanned the commits and identified these to be relevant to mqttwarn core, in chronological order.

Merging all the enhancements to the mqttwarn plugins should be straightforward. Thanks to all who contributed!

As I didn't follow the development thoroughly, I am humbly asking @jpmens to review this list and maybe acknowledge my findings. If I failed on that summary, I will be happy to learn otherwise.

[2] https://github.com/jpmens/mqttwarn/compare/develop...master

jpmens commented 4 years ago

@dlangille first of all thank you for the reminder and rest assured: mqttwarn will not simply just die on that day in January, and neither will Python2. Admittedly in terms of Python2 support ....

@amotl I'm really glad to see you back! Welcome home! :-)

Both of you: unbelievable, but I dreamed of mqttwarn and Python3 last night.

I'll review @amotl's question later in the course of the afternoon (TZ=CET) as I'm giving an Ansible training which begins in 45m.

jpmens commented 4 years ago

@amotl I couldn't resist. Yes, you're right, and as far as I can tell we're good to go.

As soon as you have something, please go ahead and commit it. You're the boss here today (and tomorrow, etc. :-) !

dlangille commented 4 years ago

@jpmens Thank you. This all came to mind over the weekend as I was replacing Python 2.7 with Python 3.6

jpmens commented 4 years ago

@dlangille due to my current interest in FreeBSD, mqttwarn will also be a first-class citizen on that platform. Together with your packaging expertise on that platform I expect no less than greatness. :-)

amotl commented 4 years ago

I've just updated the "develop" branch by integrating the changes from "master". It turned out to be a total no-brainer. git is amazing.

For the records, these are the conflicts I got:

offgrid:mqttwarn amo$ git checkout develop
offgrid:mqttwarn amo$ git merge master
CONFLICT (rename/delete): services/nma.py deleted in HEAD and renamed to mqttwarn/services/nma.py in develop. Version develop of mqttwarn/services/nma.py left in tree.
CONFLICT (file location): services/mysql_remap.py added in HEAD inside a directory that was renamed in develop, suggesting it should perhaps be moved to mqttwarn/services/mysql_remap.py.
CONFLICT (file location): services/alexa-notify-me.py added in HEAD inside a directory that was renamed in develop, suggesting it should perhaps be moved to mqttwarn/services/alexa-notify-me.py.
CONFLICT (modify/delete): mqttwarn.py deleted in develop and modified in HEAD. Version HEAD of mqttwarn.py left in tree.
Auto-merging README.md
Auto-merging .gitignore
CONFLICT (content): Merge conflict in .gitignore
Automatic merge failed; fix conflicts and then commit the result.

Resolving them was easy based on the review above and quickly looking up that the nma plugin indeed has been abandoned. As mqttwarn core (the former mqttwarn.py main script) did not receive any updates, I just ignored it (aka. git rm mqttwarn.py) when resolving the commit.

Also, the work-in-progress test harness (#332) has been integrated and as expected, all tests pass successfully. Anyone who is interested can also run them by just typing make test within the projects' root directory.

amotl commented 4 years ago

On the journey of making mqttwarn compatible with Python 3, I just went ahead after giving the "develop" branch some more love and published it to the Python Package Index (PyPI).

Read (and discuss) all about it within #127.

This is a huge leap forward. However, please note the codebase has not been touched to support Python 3 yet.

amotl commented 4 years ago

Hi there,

40fa69a finally brings mqttwarn core way closer to Python3.

Within https://github.com/jpmens/mqttwarn/issues/127#issuecomment-557944816, @dlangille offered to send a merge request for bringing all service plugins up to speed. This would be very welcome. Please make sure you are submitting it to the "develop" branch of this repository. Thanks already!

With kind regards, Andreas.

amotl commented 4 years ago

Some service modules are still based on urllib and urllib2. urllib is the hardest module to use from Python 2/3 compatible code, see also https://python-future.org/compatible_idioms.html#urllib-module.

The list of relevant service plugins are:

In order to have the code base compatible with Python2/3, we might want to either introduce the requests module or to use http://python-future.org/.

jpmens commented 4 years ago

Let's take this to requests. There's hardly anything requests cannot do, and it is easy to use.

mqttwarn modules which are not converted by their maintainers will likely fall wayside.

dgomes commented 4 years ago

instapush has been closed, so that one can be removed altogether

jpmens commented 4 years ago

@dgomes thank you and thank you for that contribution. :-)

amotl commented 4 years ago

Thanks already, @dgomes and @jpmens. I've further added 17bf2745 and 578ab704 to address more aspects with respect to Python2/3 compatibility.

amotl commented 4 years ago

Hi there,

the latest and greatest mqttwarn-0.13.2 has just been released [1]. It should be reasonably compatible with Python 3, but we will be happy to hear about anything which doesn't work properly yet -- either on Python 2 and Python 3!

In order to mitigate eventual fallout coming from this process, we want to encourage everyone to install mqttwarn from PyPI, test it using their favorite plugins and report the outcome back to us.

Thanks already!

With kind regards, Andreas.

[1] https://pypi.org/project/mqttwarn/

amotl commented 4 years ago

For verifying Python 3.8 support, we might want to check if things related to Jinja2 are still working in this environment, see also https://github.com/pallets/jinja/issues/1119.

amotl commented 4 years ago

I just added tox support for Python 3.8 and it also works flawlessly.

I would like to close this issue if I could at least get some encouraging words from @jpmens, @ckrey, @dlangille, @rgitzel, @Gulaschcowboy or maybe others who are using mqttwarn on a regular basis.

If you are able to catch some time, I will be happy to hear on which systems you are running mqttwarn successfully these days. We might want to add this to the documentation at some place.

jpmens commented 4 years ago

@amotl I can pretty much not get anything working on macOS (Python 3.7.2), neither with python setup.py install nor with pip install mqttwarn, after creating a virtual environment with python -mvenv v3; source v3/bin/activate.

A trivial “hello” to topic hello/1 and output to file:f01 doesn’t work. I’ve spent a bit debugging into the file service, and the service seems to crash out when append_newline is checked in config{}. If I comment out those lines completely, I then get

mqttwarn.services.file   ] Cannot write to file `b'/tmp/f.01'': [Errno 2] No such file or directory: "b'/tmp/f.01'"    mqttwarn.services.file   ] Cannot write to file `b'/tmp/f.01'': [Errno 2] No such file or directory: "b'/tmp/f.01'"

which makes me think something general is happening with encoding.

I’ll continue testing tomorrow.

amotl commented 4 years ago

@jpmens Thanks for your quick response and your observations. Getting things right here is very important.

something general is happening with encoding

True. I hope 0.13.7 fixes these issues through 442a51cc, now also supported by appropriate software tests.

jpmens commented 4 years ago

@amotl that fixed it, thanks

jpmens commented 4 years ago

I have tested a few mqttwarn services on macOS 10.15 (Catalina)with Python 3.7.2 and on OpenBSD 6.6-CURRENT with Python 3.7.6. I chose services I'm most familiar with, log, file, smtp, pushover, and all work as expected.

You've done a fabulous job, @amotl, and I am now closing this issue.

amotl commented 4 years ago

Thanks for testing!

rgitzel commented 4 years ago

@amotl Apologies first for not having kept up. But I was glad to see all the activity. :-)

I just gave the latest code in master a go, and made what seemed to be the simplest changes to the Dockerfile to get it to run: https://github.com/jpmens/mqttwarn/compare/master...rgitzel:try-python3-version

It runs, but fails miserably. The log is full of errors like this:

2020-01-13 05:22:45,962 WARNING  [mqttwarn.context         ] Cannot invoke alldata function wifi_values defined in wifi: Missing parentheses in call to 'print'. Did you mean print(message)? (funcs.py, line 179)

So presumably I need to get my functions running in Python 3 as well?

Has there been any migration documentation prepared? I'm wondering what else I'll need to change.

jpmens commented 4 years ago

@rgitzel that error message appears to indicate that your funcs.py file isn't Python3 compatible. It is quite likely that you will be able to get that going quickly by surrounding print statements with parentheses

print(something)
rgitzel commented 4 years ago

Upgrading my functions to Python 3 seems to have done the trick.

So, I'm seeing three times the log entries:

2020-01-14 05:06:46,521 INFO     [mqttwarn.core            ] Invoking service plugin for `log'
2020-01-14 05:06:46,521 INFO     [mqttwarn.services.log    ] wifi,Device=tasmota-garage,Router=EE:9F:DB:F1:B6:08 Signal=100 1578978406000000000
2020-01-14 05:06:46,522 INFO     [mqttwarn.core            ] Invoking service plugin for `mqttpub'
2020-01-14 05:06:46,539 INFO     [mqttwarn.core            ] Invoking service plugin for `log'
2020-01-14 05:06:46,539 INFO     [mqttwarn.services.log    ] temperature,Device=tasmota-garage,Sensor=DHT22 Temperature=-3.0 1578978406000000000
2020-01-14 05:06:46,540 INFO     [mqttwarn.core            ] Invoking service plugin for `mqttpub'
2020-01-14 05:06:46,560 INFO     [mqttwarn.core            ] Invoking service plugin for `log'
2020-01-14 05:06:46,561 INFO     [mqttwarn.services.log    ] humidity,Device=tasmota-garage,Sensor=DHT22 Humidity=95.9 1578978406000000000
2020-01-14 05:06:46,562 INFO     [mqttwarn.core            ] Invoking service plugin for `mqttpub'

Should the "Invoking..." messages be at INFO? They seem more like DEBUG.

amotl commented 1 year ago

Hi again,

GH-676 revealed that not all service plugins are compatible with Python 3 yet. On this matter, I just discovered the python3 branch by @SpotlightKid, which may have a few significant improvements in this area, see ^1.

Would you still be up for upstreaming corresponding improvements to the service plugins selectively, Christopher?

With kind regards, Andreas.

SpotlightKid commented 1 year ago

Phew, that is more than 4 years ago. I almost forgot that I did that. I certainly forgot what I did.

I'm afraid I currently don't have much time for FLOSS development and the little time I have I'd like to spend on my own projects. So I am not able to incorporate these change sino the current state of the project, sorry.

However, feel free to pick whatever you want from my branch, be it whole commits or just individual changes or bits of code.