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

Handle non atomic write method when writing on network fs with linux #991

Closed
wants to merge 1 commit into from

Conversation

jlorrain
Copy link

@jlorrain jlorrain commented Dec 16, 2020

Linux doesn't provide a sync method for network mounted fs, which cause atomicwrites to fail.
I added a failsafe to write with a basic (non atomic) method for that edge case

Fix issue #858

if attempt == 0:
except OSError as e:
# If we are writing to the network, we can't ensure atomicity
if e.errno == 22:
Copy link
Contributor

Choose a reason for hiding this comment

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

Error no. 22 is an invalid filepath, right? What does that have to do with network paths?

Copy link
Author

Choose a reason for hiding this comment

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

The goal is to handle the sync method error of atomicwrites:

File "/usr/local/lib/python3.7/site-packages/rez/vendor/atomicwrites/init.py", line 44, in _sync_directory
_proper_fsync(fd)
OSError: [Errno 22] Invalid argument

This is ugly, but atomicwrites doesn't handle properly this error itself

@nerdvegas
Copy link
Contributor

Hey @jlorrain thanks for finding this and providing a fix. I think this open_file_for_write has gotten a bit messy, I'm gonna take what you have here and put in a new PR that incorporates the same approach, so don't be alarmed if this gets closed. I'll comment with new PR details here.

@nerdvegas
Copy link
Contributor

Closing in lieu of #998

@nerdvegas nerdvegas closed this Dec 29, 2020
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.

3 participants