Skip to content

[18.0][FIX] fs_storage: fix _ls_check_connection when dealing with huge amount of files in root folder#453

Merged
OCA-git-bot merged 1 commit intoOCA:18.0from
camptocamp:18-fix-fs_storage
Mar 20, 2025
Merged

[18.0][FIX] fs_storage: fix _ls_check_connection when dealing with huge amount of files in root folder#453
OCA-git-bot merged 1 commit intoOCA:18.0from
camptocamp:18-fix-fs_storage

Conversation

@sebalix
Copy link
Copy Markdown
Contributor

@sebalix sebalix commented Mar 4, 2025

Context

We tried to configure a storage on an AzureBlobFileSystem with 3.8+M of files located at the root directory.

Issue

This storage is in read-only access, so we have to use the other option ls to check the connection. Issue is that the current ls on root folder will try to return too much file names, increasing the consumed memory, leading to a kill of the Odoo process.

Proposed solution

Perform this ls on a non-existing file, which should then raise FileNotFoundError (to check if this exception is returned by all protocols?). If nothing is raised, that also means the connection is working (as before).

EDIT: Feedback, it is working well after 2 weeks


_logger = logging.getLogger(__name__)

LS_NON_EXISTING_FILE = ".NON_EXISTING_FILE"
Copy link
Copy Markdown
Contributor Author

@sebalix sebalix Mar 4, 2025

Choose a reason for hiding this comment

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

Should it be a new config field? Should we add some random characters when using it to ensure it is non-existing on remote FS?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(even if the file exists, as soon as ls returns we are good)

@sebalix sebalix force-pushed the 18-fix-fs_storage branch from 0972547 to d510be3 Compare March 4, 2025 14:34
@sebalix sebalix force-pushed the 18-fix-fs_storage branch from d510be3 to 84a4755 Compare March 4, 2025 14:38
@sebalix sebalix marked this pull request as ready for review March 4, 2025 15:04
Copy link
Copy Markdown
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

LG

@lmignon
Copy link
Copy Markdown
Contributor

lmignon commented Mar 20, 2025

/ocabot merge patch

@OCA-git-bot
Copy link
Copy Markdown
Contributor

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

@OCA-git-bot OCA-git-bot merged commit fd82e94 into OCA:18.0 Mar 20, 2025
0 of 2 checks passed
@OCA-git-bot
Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants