irods / python-irodsclient

A Python API for iRODS
Other
63 stars 73 forks source link

Return better error on incorrectly constructing `iRODSAccess` #558

Closed alanking closed 2 months ago

alanking commented 4 months ago

If the wrong type is passed for any of the members of and iRODSAccess object, the copy() member function can fail on the call to copy.deepcopy: https://github.com/irods/python-irodsclient/blob/71d787fe1f79d81775d892c59f3e9a9f60262c78/irods/access.py#L67

I think the checks should occur in __init__: https://github.com/irods/python-irodsclient/blob/71d787fe1f79d81775d892c59f3e9a9f60262c78/irods/access.py#L59-L64

Currently, if a type such as an iRODSCollection is passed for path instead of a str (the expected type), an Exception like this can be raised when copy is invoked:

  File "/path/to/script.py", line 90, in <module>
    session.acls.set(acls)
  File "/path/to/virtual_env/lib/python3.10/site-packages/irods/manager/access_manager.py", line 135, in set
    acl = acl.copy(decanonicalize = True)
  File "/path/to/virtual_env/lib/python3.10/site-packages/irods/access.py", line 67, in copy
    other = copy.deepcopy(self)
  File "/usr/lib/python3.10/copy.py", line 172, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/lib/python3.10/copy.py", line 271, in _reconstruct
    state = deepcopy(state, memo)
  File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/lib/python3.10/copy.py", line 172, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/lib/python3.10/copy.py", line 271, in _reconstruct
    state = deepcopy(state, memo)
  File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/lib/python3.10/copy.py", line 172, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/lib/python3.10/copy.py", line 271, in _reconstruct
    state = deepcopy(state, memo)
  File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/lib/python3.10/copy.py", line 172, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/lib/python3.10/copy.py", line 271, in _reconstruct
    state = deepcopy(state, memo)
  File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/lib/python3.10/copy.py", line 172, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "/usr/lib/python3.10/copy.py", line 271, in _reconstruct
    state = deepcopy(state, memo)
  File "/usr/lib/python3.10/copy.py", line 146, in deepcopy
    y = copier(x, memo)
  File "/usr/lib/python3.10/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
  File "/usr/lib/python3.10/copy.py", line 161, in deepcopy
    rv = reductor(4)
TypeError: cannot pickle '_thread._local' object

The stacktrace is not very clear and could be avoided with a well-placed type check earlier in the process.

alanking commented 2 months ago

We could extract the path member from a list of supported types (e.g. iRODSCollection) and then raise a TypeError for other non-str types... Or, just raise a TypeError for anything but the expected str type.