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
feat(fs): add timeouts to atomic operations #3534
Conversation
@xen-orchestra/fs/src/abstract.js
Outdated
@@ -121,7 +127,7 @@ export default class RemoteHandlerAbstract { | |||
newPath: string, | |||
{ checksum = false }: Object = {} | |||
) { | |||
let p = this._rename(oldPath, newPath) | |||
let p = timeout.call(this._rename(oldPath, newPath), 10000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use the constant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p
is redefined when we have checksum : L.132
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use DEFAULT_TIMEOUT
😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad. ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add timeouts on createReadStream
and createOutputStream
as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add @xen-orchestra/fs v0.4.0
to the list of packages to release in the changelog.
CHANGELOG.md
Outdated
@@ -24,6 +24,7 @@ | |||
- xo-server-usage-report v0.7.0 | |||
- xo-server v5.29.0 | |||
- xo-web v5.29.0 | |||
- @xen-orchestra/fs v0.4.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move it on top because it's a dependency of other packages to help with the (in order) release process 🙂
5bd3e98
to
29eb787
Compare
Fixes #3467
Check list
Fixes #007
)${name} v${new version}
)Process
WiP:
(Work in Progress) if not ready to be merged