Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

self._handle is not defined in __del__ #299

Closed
bryanwweber opened this issue Dec 14, 2022 · 2 comments · Fixed by #300 or #301
Closed

self._handle is not defined in __del__ #299

bryanwweber opened this issue Dec 14, 2022 · 2 comments · Fixed by #300 or #301
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@bryanwweber
Copy link
Contributor

Hello! Thank you for this awesome package. I've been debugging some errors in pytest fixtures lately, specifically ClientMismatchErrors (which are my fault, not cloudpathlib). Due to causing this error, the self._handle attribute is never set, so when pytest tears the test down and calls __del__ on CloudPath, there is a further exception raised which pytest leaves a note about. The fix is fairly simple, it should be enough to move the definition of the self._handle attribute to the top of __init__, or else use a try/except block in __del__ to catch this situation. Are you open to a patch for this?

Sample traceback:

python3.9/site-packages/_pytest/unraisableexception.py:78: PytestUnraisableExceptionWarning: Exception ignored in: <function CloudPath.__del__ at 0x1071b9940>

  Traceback (most recent call last):
    File ".../lib/python3.9/site-packages/cloudpathlib/cloudpath.py", line 184, in __del__
      if self._handle is not None:
  AttributeError: 'S3Path' object has no attribute '_handle'

    warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))

This is also documented in pytest here: https://docs.pytest.org/en/7.1.x/how-to/failures.html#warning-about-unraisable-exceptions-and-unhandled-thread-exceptions

@pjbull pjbull added bug Something isn't working good first issue Good for newcomers labels Dec 14, 2022
@pjbull
Copy link
Member

pjbull commented Dec 14, 2022

Thanks @bryanwweber! Yes, let's move the _handle initialization to near the top of that function so we don't hit this in the future. That would be a great patch.

@bryanwweber
Copy link
Contributor Author

Thanks for the quick reply @pjbull! The fix is in #300.

pjbull pushed a commit that referenced this issue Dec 15, 2022
* Move _handle attribute up in CloudPath __init__

The _handle attribute needs to be set before any code can raise an exception. This is because if something tries to call __del__ after the exception as the process is exiting, _handle must be set to avoid an AttributeError in __del__.

Resolves #299.

* Formatting

Co-authored-by: Bryan Weber <bweber@rebelliondefense.com>
pjbull added a commit that referenced this issue Dec 15, 2022
* Move _handle attribute up in CloudPath __init__

The _handle attribute needs to be set before any code can raise an exception. This is because if something tries to call __del__ after the exception as the process is exiting, _handle must be set to avoid an AttributeError in __del__.

Resolves #299.

* Formatting

Co-authored-by: Bryan Weber <bweber@rebelliondefense.com>

Co-authored-by: Bryan Weber <bryan.w.weber@gmail.com>
Co-authored-by: Bryan Weber <bweber@rebelliondefense.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants