Isilon / isilon_sdk_python

Official generated source of the Isilon SDK Python language bindings.
36 stars 33 forks source link

Inconsistent SSLError Handling #14

Open tucked opened 5 years ago

tucked commented 5 years ago

The SDK claims support for urllib3 >= 1.15: https://github.com/Isilon/isilon_sdk_python/blob/19958108ec550865ebeb1f2a4d250322cf4681c2/setup.py#L25

In urllib3 1.22, MaxRetryError is raised instead of SSLError directly:

Made the connection pool retry on SSLError. The original SSLError is available on MaxRetryError.reason. (Issue #1112)

This means that if urllib3 >= 1.22 is used, this code is bypassed: https://github.com/Isilon/isilon_sdk_python/blob/19958108ec550865ebeb1f2a4d250322cf4681c2/isi_sdk/rest.py#L220-L222

  1. That means urllib3.exceptions.MaxRetryError is exposed directly to the caller (even though the use of urllib3 is an implementation detail).
  2. The bypassed code discards the original exception which means that the caller must parse the ApiException.reason string to figure out what happened. In a way, this makes 1 preferable.
tucked commented 5 years ago

Also,

urllib3 does not have backward-compatibility guarantees on minor releases

So, pinning urllib3 to a compatible patch release (e.g. urllib3 ~= 1.15.1) is probably advisable.