search

wernerfred / check_synology

This plugin will check a lot of different values on your Synology DiskStation.
MIT License
17 stars 25 forks source link

fix: Improve error handling when monitored appliance is not available #30

Closed amotl closed 2 years ago

amotl commented 2 years ago

Dear Frederic,

this patch aims to improve robustness of non-happy code paths after bringing in #19 the other day.

Previously, the program yielded the rather cryptic error message <built-in function get> returned NULL without setting an error and also added a traceback, not catching the exception.

Now, any such exceptions are caught and cleanly converged into the UNKNOWN state, so that the program croaks with:

UNKNOWN - timed out while connecting to remote host

Note: The improved error message will only be available when installing the latest easysnmp from the upstream repository.

With kind regards, Andreas.

Before

How it looked like on my aNag when the appliance went down.

image

After

Now it looks compact and clean again.

image

With all six sensors active, the space-saving is even more dramatic and obvious. And of course, the error message is way clearer than before.

image

amotl commented 2 years ago

Note: The improved error message timed out while connecting to remote host will only be available when installing the latest easysnmp package from its upstream repository.

I just asked @kamakazikamikaze about the state of affairs over at https://github.com/kamakazikamikaze/easysnmp/issues/147.

kamakazikamikaze commented 2 years ago

Please let me know if a full release instead of a pre-release is required. I was hoping to get a few more bugfixes out for 0.2.6 but it's more important to me that projects dependent on the enhancements are able to operate effectively.

amotl commented 2 years ago

Dear Kent,

thank you very much for your quick response on this matter. I think it is no problem to install both packages using an incantation like

pip install git+https://github.com/wernerfred/check_synology easysnmp==0.2.6a1

until 0.2.6 GA will be published [^1].

It's more important to me that projects dependent on the enhancements are able to operate effectively.

I very much appreciate that, keep up the spirit. Good luck with the next round of fixes to easysnmp!

With kind regards, Andreas.

[^1]: At the same time, this reminds us that @wernerfred might also decide to publish this package to PyPI within one of the next iterations?

wernerfred commented 2 years ago

I think it is no problem to install both packages using an incantation like

pip install git+https://github.com/wernerfred/check_synology easysnmp==0.2.6a1

Should we add a hint on this specific version in the README?

  1. At the same time, this reminds us that @wernerfred might also decide to publish this package to PyPI within one of the next iterations? ↩

No real objections on it but also never did this and low on time. I would favor a solution that is build on github actions that will release/update pipy alongside github releases (or is this a onetimer and will automatically pick up new releases from github?)

EDIT: there is one already in place and explained on official site: https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/

amotl commented 2 years ago

Dear Kent (@kamakazikamikaze),

we can confirm easysnmp-0.2.6a1 works very well for us.

Please let me know if a full release instead of a pre-release is required.

Indeed, a full release would be needed in order to express it in the setup.py file as easysnmp>=0.2.6. I don't think it will work otherwise, unless the user specifies pip install --pre.

Do you see any chance to publish a full 0.2.6 release of easysnmp? I think the community will be really grateful.

With kind regards, Andreas.

amotl commented 2 years ago

Dear Werner,

Should we add a hint on this specific version in the README?

I think all will be fine as soon as easysnmp-0.2.6 GA is published. When adjusting setup.py appropriately, we will not need any specific admonition in the README file.

What about publishing this package to PyPI?

I would favor a solution that is build on github actions that will release/update pipy alongside github releases.

I hear you. We have a diverse set of release procedures on our Python repositories, also based on GHA. When I can find some time, I will pick the best-of-breed solution and submit a corresponding patch here. Maybe next winter.

With kind regards, Andreas.

kamakazikamikaze commented 2 years ago

I plan to publish 0.2.6 this weekend. I finally got around to addressing an SNMP v3 issue while sneaking in some proper Net-SNMP library cleanup. Just know that I will likely be moving the repo to the new org I created earlier this year.

kamakazikamikaze commented 2 years ago

I fast-tracked the publication. easysnmp-0.2.6 is now available via pip.

wernerfred commented 2 years ago

I fast-tracked the publication. easysnmp-0.2.6 is now available via pip.

@all-contributors pls add @kamakazikamikaze for plugin

Thx for your support here (regarding easysnmp) and quick reactions!

allcontributors[bot] commented 2 years ago

@wernerfred

I've put up a pull request to add @kamakazikamikaze! :tada: