Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove support for migrating from tahoe-lafs magic folders. #401

Closed
tomprince opened this issue Jun 7, 2021 · 7 comments
Closed

Remove support for migrating from tahoe-lafs magic folders. #401

tomprince opened this issue Jun 7, 2021 · 7 comments

Comments

@tomprince
Copy link
Contributor

This is not required for the first release, and doesn't handle migrating anything other than the folder name and path.

@meejah
Copy link
Collaborator

meejah commented Jun 7, 2021

Why do we need to remove it?

@meejah
Copy link
Collaborator

meejah commented Jun 7, 2021

(Anyway what else would be migrated? We have to create the Snapshots .. but those should be created by the uploader, provided the magic-path is still the same one that contains all the files).

@tomprince
Copy link
Contributor Author

Why do we need to remove it?

#400 (comment)

@tomprince
Copy link
Contributor Author

Why do we need to remove it?

To expand on #400 (comment), having code in the repository that is not need for privatestorage imposes a (probably small) maintenance burden. I'm inclined to aggressively remove code that doesn't support privatestorage, when we discover it, rather than trying to fix it, given our timeline.

(Anyway what else would be migrated? We have to create the Snapshots .. but those should be created by the uploader, provided the magic-path is still the same one that contains all the files).

One thought that occurs to me is that we may want to store some metadata to the tahoe folders, that allows each machine to migrate their local magic folder, without needing to do the invite dance between all of them.

@meejah
Copy link
Collaborator

meejah commented Jun 8, 2021

This is not just a PrivateStorage project. I don't think we need to remove working code just because PrivateStorage doesn't need it immediately.

I think the best course is to just file a bug about any case that isn't working. We can ignore the bug for the immediate needs of PrivateStorage if it indeed doesn't require magic-folder migrate so no need to fix it.

@exarkun
Copy link
Member

exarkun commented Jun 8, 2021

This is not required for the first release, and doesn't handle migrating anything other than the folder name and path.
(Anyway what else would be migrated? We have to create the Snapshots .. but those should be created by the uploader, provided the magic-path is still the same one that contains all the files).

It sounds like the migrator might be more complete than it appears at first glance. I see that there are some user-facing docs that mention migration (docs/usage.rst) but they're pretty brief and they don't explain what to expect of the process. From the title of this ticket, it seems like Tom thought it was an incomplete feature. From what meejah is saying, it sounds like it is in fact more or less complete (maybe it has bugs but it's not missing big chunks of functionality).

Given that, I think we probably shouldn't remove it. It would be good to clarify its status in the project documentation. While that isn't on the hot-path for PrivateStorage's goals, that kind of project documentation might be an unavoidable requirement in practice if we want new contributors to be able to find there way around.

@meejah meejah changed the title Remove partial support for migrating from tahoe-lafs magic folders. Remove support for migrating from tahoe-lafs magic folders. Jun 10, 2021
@meejah meejah closed this as completed Jun 18, 2021
@tomprince
Copy link
Contributor Author

I ended up looking at this code again due to #509 and noticed that we are using the dircap's from tahoe-lafs which are incompatible with new magic-folder. Given that, I think this command is currently completely broken. We will fail trying to read any existing snapshots from peers, and any snapshots we take will cause peers to fail to download our snapshots. (Although #420 will avoid us trying to do either of those things.)

Given that, I think it makes sense to remove this command.

@tomprince tomprince reopened this Jul 28, 2021
@meejah meejah closed this as completed Aug 27, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants