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

Defend from AttributeErrors in CloudPath.__del__ #418

Merged

Conversation

bryanwweber
Copy link
Contributor

If the CloudPath Client instance is incomplete, it may not have a file_cache_mode attribute. Using getattr defensively prevents the AttributeError from causing del to exit early.

Closes #372


Contributor checklist:

  • I have read and understood CONTRIBUTING.md
  • Confirmed an issue exists for the PR, and the text Closes #issue appears in the PR summary (e.g., Closes #123).
  • Confirmed PR is rebased onto the latest base
  • Confirmed failure before change and success after change
  • Linting passes locally
  • Tests pass locally
  • Updated HISTORY.md with the issue that is addressed and the PR you are submitting. If the top section is not `## UNRELEASED``, then you need to add a new section to the top of the document for your change.

If the CloudPath Client instance is incomplete, it may not have a
file_cache_mode attribute. Using getattr defensively prevents the
AttributeError from causing __del__ to exit early.
cloudpathlib/cloudpath.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented Apr 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.4%. Comparing base (8a4198a) to head (2bb26ff).

❗ Current head 2bb26ff differs from pull request most recent head bf5fe37. Consider uploading reports for the commit bf5fe37 to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #418     +/-   ##
========================================
- Coverage    93.8%   93.4%   -0.4%     
========================================
  Files          23      23             
  Lines        1643    1644      +1     
========================================
- Hits         1542    1537      -5     
- Misses        101     107      +6     
Files Coverage Δ
cloudpathlib/cloudpath.py 93.5% <100.0%> (-0.4%) ⬇️

... and 2 files with indirect coverage changes

cloudpathlib/cloudpath.py Outdated Show resolved Hide resolved
@jayqi jayqi changed the base branch from master to 418-live-tests April 1, 2024 19:45
Copy link
Member

@jayqi jayqi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Merging this into new branch 418-live-tests for live tests.

@jayqi jayqi merged commit 8ce39c3 into drivendataorg:418-live-tests Apr 1, 2024
5 of 20 checks passed
@bryanwweber
Copy link
Contributor Author

Thank you, and thank you for addressing the nit ☺️

jayqi added a commit that referenced this pull request Apr 1, 2024
* Defend from AttributeErrors in CloudPath.__del__

If the CloudPath Client instance is incomplete, it may not have a
file_cache_mode attribute. Using getattr defensively prevents the
AttributeError from causing __del__ to exit early.

* Update HISTORY

* Fix tests by fixing a copy-paste mistake

* Use None instead of object as dummy client.

---------

Co-authored-by: Bryan Weber <bryan.w.weber@gmail.com>
Co-authored-by: Jay Qi <jayqi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AttributeError when __del__ is called on S3Client that errors during __init__
3 participants