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

Fix filelock if flock not supported #2402

Merged
merged 1 commit into from
Jul 24, 2024
Merged

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Jul 19, 2024

Fix #2399 (cc @jeffliu-LL)

On unix, filelock checks if fcntl is available. If it is, filelock.Filelock is in fact a UnixFileLock which uses fcntl.flock to acquire/release the lock. Since it's a system call, the lock is supposed to be much more robust (e.g. does not rely on file existence or not). The problem is that if fcntl is available but at least one partition or filesystem does not support it, a NotImplementedError exception is raised with message "FileSystem does not appear to support flock; use SoftFileLock instead".

This PR fixes usage of filelok by catching this NotImplementedError when it happens and defaulting back to a SoftFileLock. This is a weaker solution but for our use case it should be fine (+ it's already the filelock used by default on win32).

@Wauplin Wauplin requested review from julien-c and LysandreJik July 19, 2024 11:42
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Ok! I learned a few things reading the description, thanks for making it so clear!

@Wauplin
Copy link
Contributor Author

Wauplin commented Jul 24, 2024

I've learnt a few things myself and will probably have forgotten them next time I need to come back to this PR! Hope the PR description will save us then 😄

@Wauplin Wauplin merged commit f8cac53 into main Jul 24, 2024
17 checks passed
@Wauplin Wauplin deleted the 2399-fix-filelock-if-no-flock branch July 24, 2024 14:18
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.

v0.23 download method breaks on filesystems that don't have flock enabled
3 participants