[16.0][IMP] fs_storage: invalidate filesystem cache when connection fails

[JordiMForgeFlow]

Fixes https://github.com/OCA/storage/issues/319

CC @LoisRForgeFlow

[JordiMForgeFlow]

@lmignon after trying using fs.exists and looking into more detail, I have put back using fs.ls.

The fs.exists does not raise an error, but simply returns true or false. Also, it basically calls the method info which ends up calling the method fs.ls but with detail=True, which is worse than what we had. (code can be seen here: https://filesystem-spec.readthedocs.io/en/latest/_modules/fsspec/spec.html#AbstractFileSystem)

[lmignon]

@lmignon after trying using fs.exists and looking into more detail, I have put back using fs.ls.

The fs.exists does not raise an error, but simply returns true or false. Also, it basically calls the method info which ends up calling the method fs.ls but with detail=True, which is worse than what we had. (code can be seen here: https://filesystem-spec.readthedocs.io/en/latest/_modules/fsspec/spec.html#AbstractFileSystem)

My concern is about the performance cost when the root directory contains hundred of thousand files... To limit the scope of the ls command, it could be performed on a specific path to a marker file. What do you thing about?

[simahawk]

@lmignon after trying using fs.exists and looking into more detail, I have put back using fs.ls. The fs.exists does not raise an error, but simply returns true or false. Also, it basically calls the method info which ends up calling the method fs.ls but with detail=True, which is worse than what we had. (code can be seen here: filesystem-spec.readthedocs.io/en/latest/_modules/fsspec/spec.html#AbstractFileSystem)

My concern is about the performance cost when the root directory contains hundred of thousand files... To limit the scope of the ls command, it could be performed on a specific path to a marker file. What do you thing about?

I agree. We should probably create a file on 1st check (eg: .odoo_fs_storage_$id.marker).

[sebalix]

Hello @JordiMForgeFlow , when testing this PR, I get the following notification when using the Test Connection button:

Without the PR the connection is working well.

[JordiMForgeFlow]

@sebalix it seems to be coming from an issue in FS. The Odoo filesystem uses the local filesystem class. There was an issue with the ls for files (https://github.com/fsspec/filesystem_spec/pull/1479). It is already merged but not released yet, I have asked them when should we expect it.

[simahawk]

@sebalix can you try by installing it from https://github.com/fsspec/filesystem_spec ?

[sebalix]

Regarding connection failure, I replaced the use of the deprecated functions in edi_storage_oca there: https://github.com/OCA/edi-framework/pull/65#issuecomment-1923307964 Copying my comment:

I'm suspecting (but I'm not sure) it was the use of these deprecated methods that triggers connection issues over time. These methods are decorated, and it could be that the decorator is keeping an old reference to self.__fs having a socket that gets closed after some time. That could explain why the "Test connection" button of fs_storage provides the expected result, while the EDI jobs are failing even when requeued. This will be tested anyway.

Maybe it's not related at all.

[lmignon]

/ocabot merge minor

[OCA-git-bot]

What a great day to merge this nice PR. Let's do it! Prepared branch 16.0-ocabot-merge-pr-320-by-lmignon-bump-minor, awaiting test results.

[OCA-git-bot]

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