finish06 / pyunifi

https://unifi-sdn.ubnt.com/
MIT License
223 stars 99 forks source link

InsecureRequestWarning appearing while liburl3 is configured to mute them #75

Open JamesMenetrey opened 2 years ago

JamesMenetrey commented 2 years ago

Hey Pyunifi developers,

I have set the flag ssl_verify to False as x.509 certificate verification is not a concern in my environment. While this flag is disabled, I noticed that the script's output is spammed warnings of type InsecureRequestWarning. In the context of urllib3, this is a normal behavior, as stated in the official documentation. This may be disabled using the following code:

import urllib3
urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)

Nonetheless, after implementing this code in my project, the warnings are still triggered. After diving into Pyunifi, I discovered that this issue was bound to the following instructions:

https://github.com/finish06/pyunifi/blob/78174d580633edc5d164648167f37bb3f04e1416/pyunifi/controller.py#L109-L110

Indeed, Pyunifi reactivates the warnings in case the flag ssl_verify is set to False.

From a developer perspective that consumes your library, I have the feeling the library forces me to trigger that warning in the underlying library urllib3 and mask the possibility to use the built-in feature of that underlying library to mute the warnings. In my opinion, this choice must be made by the consumer of the end application, rather than the library, to allow better extensibility.

I propose two solutions for improving that case: c

The latter solution keeps the library backward compatible in terms of behavior, but on the other hand, I think these two instructions don't add any values to Pyunifi.

If you are satisfied with one of these solutions, I can submit a pull request to perform those changes. If not so, I'm looking forward to discussing to find a better alternative to those unmaskable warnings.

Many thanks! Cheers

RobinCutshaw commented 1 year ago

I just ran into this issue as well (trying to remove the warning message). I would request that, if you don't make the changes suggested by JamesMenetrey, the "default" parameter be set to "ignore".