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

ENH: swallow exceptions in __del__ if lgr or os.fdopen already unbound #3851

Merged
merged 1 commit into from Nov 1, 2019

Conversation

yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented Oct 31, 2019

E.g. was happening during

$> rm 100206_3T_Diffusion_unproc.json; datalad download-url s3://hcp-openaccess/HCP_1200/100206/.xdlm/100206_3T_Diffusion_unproc.json
[INFO   ] Downloading 's3://hcp-openaccess/HCP_1200/100206/.xdlm/100206_3T_Diffusion_unproc.json' into '/tmp/testds'
[INFO   ] S3 session: Connecting to the bucket hcp-openaccess with authentication
download_url(ok): /tmp/testds/100206_3T_Diffusion_unproc.json (file)
add(ok): 100206_3T_Diffusion_unproc.json (file)
[WARNING] Registering /tmp/testds/100206_3T_Diffusion_unproc.json with s3://hcp-openaccess/HCP_1200/100206/.xdlm/100206_3T_Diffusion_unproc.json failed: AnnexBatchCommandError: command 'addurl'
Error, annex reported failure for addurl (url='s3://hcp-openaccess/HCP_1200/100206/.xdlm/100206_3T_Diffusion_unproc.json'): {'command': 'addurl', 'success': False, 'file': '100206_3T_Diffusion_unproc.json'} [annexrepo.py:add_url_to_file:2261]
action summary:
  add (ok: 1)
  download_url (ok: 1)
  save (notneeded: 1)
Exception ignored in: <function BatchedAnnexes.__del__ at 0x7f45502c3840>
Traceback (most recent call last):
  File "/home/yoh/proj/datalad/datalad/datalad/support/annexrepo.py", line 3646, in __del__
  File "/home/yoh/proj/datalad/datalad/datalad/support/annexrepo.py", line 3643, in close
  File "/home/yoh/proj/datalad/datalad/datalad/support/annexrepo.py", line 3815, in close
  File "/usr/lib/python3.7/logging/__init__.py", line 1370, in debug
  File "/usr/lib/python3.7/logging/__init__.py", line 1626, in isEnabledFor
TypeError: 'NoneType' object is not callable
Exception ignored in: <function BatchedAnnex.__del__ at 0x7f45502c3c80>
Traceback (most recent call last):
  File "/home/yoh/proj/datalad/datalad/datalad/support/annexrepo.py", line 3795, in __del__
  File "/home/yoh/proj/datalad/datalad/datalad/support/annexrepo.py", line 3815, in close
  File "/usr/lib/python3.7/logging/__init__.py", line 1370, in debug
  File "/usr/lib/python3.7/logging/__init__.py", line 1626, in isEnabledFor
TypeError: 'NoneType' object is not callable

def __del__(self):
try:
self.close()
except TypeError as exc:
Copy link
Contributor

@kyleam kyleam Oct 31, 2019

Choose a reason for hiding this comment

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

nit: No need for as exc.

# TODO: Why was this commented out?
# @auto_repr
class BatchedAnnexes(dict):
class BatchedAnnexes(dict, SafeDelCloseMixin):
Copy link
Contributor

@kyleam kyleam Oct 31, 2019

Choose a reason for hiding this comment

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

It doesn't matter in this particular case, but the base class dict should be the right-most argument.

Copy link
Member Author

@yarikoptic yarikoptic Oct 31, 2019

Choose a reason for hiding this comment

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

thanks for the comments -- amended an force pushed! Need to learn why dict should be the rightmost

Copy link
Contributor

@kyleam kyleam Oct 31, 2019

Choose a reason for hiding this comment

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

Oh no, hopefully on further reflection you don't disagree with my statement then :]

I'm making the claim that SafeDelCloseMixin should have higher precedence in the resolution chain, even if by design it is not intended define any methods that overlap with dict.

class M():
    def blah(self):
        return "M"


class A():
    def blah(self):
        return "A"


class B0(A, M):
    pass


class B1(M, A):
    pass


print(B0.__mro__)  # (<class '__main__.B0'>, <class '__main__.A'>, <class '__main__.M'>, <class 'object'>)
print(B0().blah()) # A
print(B1.__mro__)  # (<class '__main__.B1'>, <class '__main__.M'>, <class '__main__.A'>, <class 'object'>)
print(B1().blah()) # M

Copy link
Member Author

@yarikoptic yarikoptic Oct 31, 2019

Choose a reason for hiding this comment

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

;) so let's keep as is now ;) but I guess that is why I typically saw mixins last, since they would then get lower precedence, BUT still probably would be invoked within MRO deeper (typically we call super first while inheriting) even when multiple subclasses (re)define the same method

E.g. was happening during

	$> rm 100206_3T_Diffusion_unproc.json; datalad download-url s3://hcp-openaccess/HCP_1200/100206/.xdlm/100206_3T_Diffusion_unproc.json
	[INFO   ] Downloading 's3://hcp-openaccess/HCP_1200/100206/.xdlm/100206_3T_Diffusion_unproc.json' into '/tmp/testds'
	[INFO   ] S3 session: Connecting to the bucket hcp-openaccess with authentication
	download_url(ok): /tmp/testds/100206_3T_Diffusion_unproc.json (file)
	add(ok): 100206_3T_Diffusion_unproc.json (file)
	[WARNING] Registering /tmp/testds/100206_3T_Diffusion_unproc.json with s3://hcp-openaccess/HCP_1200/100206/.xdlm/100206_3T_Diffusion_unproc.json failed: AnnexBatchCommandError: command 'addurl'
	Error, annex reported failure for addurl (url='s3://hcp-openaccess/HCP_1200/100206/.xdlm/100206_3T_Diffusion_unproc.json'): {'command': 'addurl', 'success': False, 'file': '100206_3T_Diffusion_unproc.json'} [annexrepo.py:add_url_to_file:2261]
	action summary:
	  add (ok: 1)
	  download_url (ok: 1)
	  save (notneeded: 1)
	Exception ignored in: <function BatchedAnnexes.__del__ at 0x7f45502c3840>
	Traceback (most recent call last):
	  File "/home/yoh/proj/datalad/datalad/datalad/support/annexrepo.py", line 3646, in __del__
	  File "/home/yoh/proj/datalad/datalad/datalad/support/annexrepo.py", line 3643, in close
	  File "/home/yoh/proj/datalad/datalad/datalad/support/annexrepo.py", line 3815, in close
	  File "/usr/lib/python3.7/logging/__init__.py", line 1370, in debug
	  File "/usr/lib/python3.7/logging/__init__.py", line 1626, in isEnabledFor
	TypeError: 'NoneType' object is not callable
	Exception ignored in: <function BatchedAnnex.__del__ at 0x7f45502c3c80>
	Traceback (most recent call last):
	  File "/home/yoh/proj/datalad/datalad/datalad/support/annexrepo.py", line 3795, in __del__
	  File "/home/yoh/proj/datalad/datalad/datalad/support/annexrepo.py", line 3815, in close
	  File "/usr/lib/python3.7/logging/__init__.py", line 1370, in debug
	  File "/usr/lib/python3.7/logging/__init__.py", line 1626, in isEnabledFor
	TypeError: 'NoneType' object is not callable
@codecov
Copy link

@codecov codecov bot commented Oct 31, 2019

Codecov Report

Merging #3851 into 0.11.x will decrease coverage by 50.84%.
The diff coverage is 60%.

Impacted file tree graph

@@             Coverage Diff             @@
##           0.11.x    #3851       +/-   ##
===========================================
- Coverage   80.37%   29.52%   -50.85%     
===========================================
  Files         256      255        -1     
  Lines       33965    33956        -9     
===========================================
- Hits        27300    10027    -17273     
- Misses       6665    23929    +17264
Impacted Files Coverage Δ
datalad/support/annexrepo.py 48.07% <60%> (-9.69%) ⬇️
datalad/customremotes/tests/__init__.py 0% <0%> (-100%) ⬇️
datalad/tests/test_protocols.py 0% <0%> (-100%) ⬇️
datalad/support/tests/test_stats.py 0% <0%> (-100%) ⬇️
datalad/tests/test_constraints.py 0% <0%> (-100%) ⬇️
datalad/support/tests/test_ansi_colors.py 0% <0%> (-100%) ⬇️
datalad/distribution/tests/test_subdataset.py 0% <0%> (-100%) ⬇️
datalad/interface/tests/test_annotate_paths.py 0% <0%> (-100%) ⬇️
datalad/cmdline/tests/__init__.py 0% <0%> (-100%) ⬇️
datalad/tests/utils_testdatasets.py 0% <0%> (-100%) ⬇️
... and 194 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3da12a0...c031806. Read the comment docs.

@kyleam kyleam merged commit c031806 into datalad:0.11.x Nov 1, 2019
2 of 5 checks passed
@kyleam
Copy link
Contributor

@kyleam kyleam commented Nov 1, 2019

The appveyor failure was the codecov failure that we've been seeing occasionally.

kyleam added a commit to kyleam/datalad that referenced this issue Nov 1, 2019
0.11.x's c031806 (ENH: swallow exceptions in __del__ if lgr or
os.fdopen already unbound, dataladgh-3851) defined a SafeDelCloseMixin class
in annexrepo.py and used it as a base class for BatchedAnnex and
BatchedAnnexes.  On master, 9d714f5 (2019-04-08) relocated much of
the core functionality for BatchedAnnex to cmd.py as BatchedCommand.

SafeDelCloseMixin is relevant for BatchedCommand (which is used in a
spot other than annexrepo.py), so move SafeDelCloseMixin to cmd.py and
drop BatchedCommand's __del__() method.

Conflicts:
    datalad/support/annexrepo.py
@yarikoptic yarikoptic deleted the enh-moresilent-del branch Nov 12, 2019
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.

None yet

2 participants