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

blockstore: allow to pass a file to write in #323

Merged
merged 2 commits into from
Aug 1, 2022

Conversation

MichaelMure
Copy link
Contributor

@MichaelMure MichaelMure commented Jul 27, 2022

This allow the caller to control the file lifecycle and implement a cleaning strategy. An example would be:

  • open a file in a temporary folder
  • if the write suceed, close the file and move it to its final destination
  • on error, remove the file

If the underlying filesystem support it, an anonymous file can be used instead: open then immediately delete, use the file descriptor to write. A crash before the end of the write process would release the used storage automatically.

Not a breaking change, only introduce an extra function.

This allow the caller to control the file lifecycle and implement a cleaning strategy.
An example would be:
- open a file in a temporary folder
- if the write suceed, close the file and move it to its final destination
- on error, remove the file

If the underlying filesystem support it, an anonymous file can be used instead: open
then immediately delete, use the file descriptor to write. A crash before the end of the
write process would release the used storage automatically.
Copy link
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

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

Imo we should rename OpenReadWrite to OpenReadWritePath and OpenReadWriteFile to OpenReadWrite but I don't think it's worth uselessly breaking consumers.

LGTM

v2/blockstore/readwrite.go Outdated Show resolved Hide resolved
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

👍

@rvagg rvagg merged commit ed56551 into ipld:master Aug 1, 2022
@rvagg
Copy link
Member

rvagg commented Aug 1, 2022

aaaaactually, I probably should have asked for tests - @MichaelMure would you mind taking a look at the current test suite and see if you can extend one of them to control the file from outside rather than passing the path?

rvagg added a commit that referenced this pull request Aug 12, 2022
This allow the caller to control the file lifecycle and implement a cleaning strategy.
An example would be:
- open a file in a temporary folder
- if the write suceed, close the file and move it to its final destination
- on error, remove the file

If the underlying filesystem support it, an anonymous file can be used instead: open
then immediately delete, use the file descriptor to write. A crash before the end of the
write process would release the used storage automatically.

Co-authored-by: Rod Vagg <rod@vagg.org>
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