search

OCA / storage

GNU Affero General Public License v3.0
70 stars 159 forks source link

[IMP] fs_storage: use fs.info() to check for the marker file #334

Closed nilshamerlinck closed 7 months ago

nilshamerlinck commented 7 months ago

I'm facing an issue with https://github.com/OCA/storage/pull/320 and a SFTP storage:

  File "/odoo/external-src/storage/fs_storage/models/fs_storage.py", line 293, in fs
    self._check_connection(self.__fs)
  File "/odoo/external-src/storage/fs_storage/models/fs_storage.py", line 372, in _check_connection
    marker_file = fs.ls(marker_file_name, detail=False)
  File "/home/odoo/.local/lib/python3.10/site-packages/fsspec/implementations/sftp.py", line 127, in ls
    stats = [self._decode_stat(stat, path) for stat in self.ftp.listdir_iter(path)]
  File "/home/odoo/.local/lib/python3.10/site-packages/fsspec/implementations/sftp.py", line 127, in <listcomp>
    stats = [self._decode_stat(stat, path) for stat in self.ftp.listdir_iter(path)]
  File "/usr/local/lib/python3.10/site-packages/paramiko/sftp_client.py", line 278, in listdir_iter
    t, msg = self._request(CMD_OPENDIR, path)
  File "/usr/local/lib/python3.10/site-packages/paramiko/sftp_client.py", line 857, in _request
    return self._read_response(num)
  File "/usr/local/lib/python3.10/site-packages/paramiko/sftp_client.py", line 909, in _read_response
    self._convert_status(msg)
  File "/usr/local/lib/python3.10/site-packages/paramiko/sftp_client.py", line 942, in _convert_status
    raise IOError(text)
OSError: Not a directory

That's because of using fs.ls() to check for the existing of the marker file, see here. The sftp implementation of that method does not (yet) support files: see here.

I understand that there was a rationale behind that choice, here.

But I think it's actually fine to use fs.info(), as underlying implementations override it with efficiency in mind:

lmignon commented 7 months ago

/ocabot merge patch

OCA-git-bot commented 7 months ago

Hey, thanks for contributing! Proceeding to merge this for you. Prepared branch 16.0-ocabot-merge-pr-334-by-lmignon-bump-patch, awaiting test results.

OCA-git-bot commented 7 months ago

Congratulations, your PR was merged at 929389d197a001ab6ec44a4e9972e7bf6007a5ef. Thanks a lot for contributing to OCA. ❤️