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

[WIP][PoC] analyze MFS Close() #5173

Closed
wants to merge 2 commits into from
Closed

[WIP][PoC] analyze MFS Close() #5173

wants to merge 2 commits into from

Conversation

schomatis
Copy link
Contributor

@schomatis schomatis commented Jul 1, 2018

Do not merge.


[WIP][PoC] coreunix: remove MFS `Close()` from `Finalize()`

I would tend to think that this is handled through `closeChild()` and
`Directory`'s `Flush()` (if responsibly called).

[WIP][PoC] mfs: remove `Close()` from `Root()`

And the functions of its only consumer, IPNS.

The first commit passes the tests (Jenkins doesn't count), the second one doesn't, which make sense: IPNS doesn't use the closeChild() and related functions (I'm imagining), modifications are not signaled to MFS as the ipfs files commands do (MFS files are just being modified by the filesystem) so it does need an explicit Close() call.

If this reasoning is correct we can eliminate the Close() call from the coreunix package (first commit) and document the function as being used only by IPNS, this will largely clarify its relationship with closeChild().

To do: address this in #5092.

I would tend to think that this is handled through `closeChild()` and
`Directory`'s `Flush()` (if responsibly called).

License: MIT
Signed-off-by: Lucas Molas <schomatis@gmail.com>
@schomatis schomatis added status/in-progress In progress topic/files Topic files topic/MFS Topic MFS labels Jul 1, 2018
@schomatis schomatis added this to the Files API Documentation milestone Jul 1, 2018
@schomatis schomatis self-assigned this Jul 1, 2018
@schomatis schomatis requested a review from Kubuxu as a code owner July 1, 2018 18:13
And the functions of its only consumer, IPNS.

License: MIT
Signed-off-by: Lucas Molas <schomatis@gmail.com>
@schomatis
Copy link
Contributor Author

This is strange, the second commit is actually passing the online tests, but in my machine TestMultipleDirs fails. Are the FUSE tests disabled here?

@schomatis
Copy link
Contributor Author

Yes, they are.

@schomatis schomatis changed the title [WIP][PoC] coreunix: remove MFS Close() from Finalize() [WIP][PoC] analyze MFS Close() Jul 1, 2018
@eingenito
Copy link
Contributor

@schomatis is it safe to close this PR? It sounds like maybe it was for CI testing?

@schomatis
Copy link
Contributor Author

Sure, thanks for the remainder!

@schomatis schomatis closed this Dec 5, 2018
@ghost ghost removed the status/in-progress In progress label Dec 5, 2018
@schomatis schomatis deleted the poc/mfs/remove-root-close branch December 5, 2018 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/files Topic files topic/MFS Topic MFS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants