Fix tqdm callback garbage collection

[nicholasjng]

Calling close() on TqdmCallback.tqdm reliably results in an exception on interpreter exit. Debug prints show that in such cases, the value of tqdm is not, as expected, the progress bar instance, but rather the no_op bound method, which does not have a close attribute.

This change checks attribute existence of close() first before calling it on the class. Checking this way does not require a tqdm import.

[nicholasjng]

Repro (scoped to LakeFSFileSystem, but I guess this also applies to other filesystems):

from fsspec.callbacks import TqdmCallback
from lakefs_spec import LakeFSFileSystem


def upload():
    fs = LakeFSFileSystem()

    res2 = fs.get_file(".sandbox/mnist.py", "quickstart/main/mnist.py", precheck=False, callback=TqdmCallback())
    print(f"LakeFS backend result: {res2}")

if __name__ == "__main__":
    upload()

Results in:

Traceback (most recent call last):
  File "/Users/nicholasjunge/Workspaces/python/lakefs-spec/.sandbox/blockstore.py", line 14, in <module>
    upload()
  File "/Users/nicholasjunge/Workspaces/python/lakefs-spec/.sandbox/blockstore.py", line 8, in upload
    res2 = fs.get_file(".sandbox/mnist.py", "quickstart/main/mnist.py", precheck=False, callback=TqdmCallback())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/nicholasjunge/Workspaces/python/lakefs-spec/src/lakefs_spec/spec.py", line 345, in get_file
    super().get_file(rpath, lpath, callback=callback, outfile=outfile, **kwargs)
  File "/Users/nicholasjunge/Workspaces/python/lakefs-spec/venv/lib/python3.11/site-packages/fsspec/spec.py", line 887, in get_file
    elif self.isdir(rpath):
         ^^^^^^^^^^^^^^^^^
  File "/Users/nicholasjunge/Workspaces/python/lakefs-spec/venv/lib/python3.11/site-packages/fsspec/spec.py", line 698, in isdir
    return self.info(path)["type"] == "directory"
           ^^^^^^^^^^^^^^^
  File "/Users/nicholasjunge/Workspaces/python/lakefs-spec/src/lakefs_spec/spec.py", line 370, in info
    repository, ref, resource = parse(path)
                                ^^^^^^^^^^^
  File "/Users/nicholasjunge/Workspaces/python/lakefs-spec/src/lakefs_spec/util.py", line 104, in parse
    raise ValueError(
ValueError: expected path with structure lakefs://<repo>/<ref>/<resource>, got '.sandbox/mnist.py'
Exception ignored in: <function TqdmCallback.__del__ at 0x1027f2ca0>
Traceback (most recent call last):
  File "/Users/nicholasjunge/Workspaces/python/lakefs-spec/venv/lib/python3.11/site-packages/fsspec/callbacks.py", line 234, in __del__
    self.tqdm.close()
    ^^^^^^^^^^^^^^^
AttributeError: 'function' object has no attribute 'close'

Adding the mentioned print statements print(type(self.tqdm)) and print(self.tqdm.__name__) in TqdmCallback.__del__ prints <class 'method'> and no_op for me, which I do not understand.

[nicholasjng]

However, the progress bar is not shown in the traceback, which indicates that it is successfully garbage collected before the callback itself is. I'd very much welcome another opinion on this.

[skshetry]

I have a proposal in #1460 to make callbacks a context manager. Ideally, we should not depend on __del__ at all. :)

[nicholasjng]

Sweet! If you could also migrate the tqdm closing to the proposed TqdmCallback.close(), then that can go ahead in place of this PR.

[martindurant]

@skshetry , I leave it up to you whether to include the small change here in your PR. I'd be happy to merge this otherwise.

[skshetry]

Please feel free (to merge). I have no issues with this PR.