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.

Sign up for GitHub

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

Jump to bottom

Introducing EncodedFSKeystore with base32 encoding #6012

Closed
wants to merge 1 commit into from
Closed

Introducing EncodedFSKeystore with base32 encoding #6012

wants to merge 1 commit into from

Conversation

AuHau
Copy link
Member

@AuHau AuHau commented Feb 22, 2019

Encoding the key's filename with base32 introduces coherent behavior
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover, it allows a wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

Tightly coupled with migration: ipfs/fs-repo-migrations#84

@AuHau AuHau requested a review from Kubuxu as a code owner February 22, 2019 00:14
@AuHau AuHau force-pushed the feat/encoding-key-names branch 2 times, most recently from 1216752 to e96bbcc Compare February 22, 2019 02:03
@AuHau
Copy link
Member Author

AuHau commented Feb 22, 2019

The failing test does not seem to be related to this PR.

Btw. I started to really like this feature, because it will enable to namespace the key's names like: websites/adam-uhlir.me

@AuHau AuHau mentioned this pull request Feb 22, 2019
Closed
@Stebalien Stebalien added status/in-progress In progress need/review Needs a review labels Feb 26, 2019
@Stebalien
Copy link
Member

Thanks! @Kubuxu can you take a look at this? If it looks good, we can merge it after the release freeze (although we may want to coordinate with #5870 as that will also require a migration).

@Kubuxu
Copy link
Member

Kubuxu commented Feb 27, 2019

Agree with after release, also I would wait with merging/migration until we can collect everything we might what to migrate.

@AuHau AuHau mentioned this pull request Mar 14, 2019
Closed
@AuHau
Copy link
Member Author

AuHau commented Mar 14, 2019

Fixed conflict.

Also one question, I have used mbase for the encoding/decoding instead of directly using base32. What do you think about it, is it alright? It sort of made sense to me...

@Stebalien
Copy link
Member

For consistency, it might be better to just use base32. Using multibase doesn't buy us anything in this case.

@AuHau
Introducing EncodedFSKeystore with base32 encoding (ipfs#5947)
a6b4745 
Encoding the key's filename with base32 introduces coherent behaviour
across different platforms and their case-sensitive/case-insensitive
file-systems. Moreover it allows wider character set to be used for the
name of the keys as the original restriction for special FS's characters
(e.g. '/', '.') will not apply.

License: MIT
Signed-off-by: Adam Uhlir <uhlir.a@gmail.com>
@Kubuxu
Copy link
Member

Kubuxu commented Apr 29, 2019

The only blocker is migrations at this point.

@AuHau
Copy link
Member Author

AuHau commented Apr 29, 2019

Migration is almost ready at: ipfs/fs-repo-migrations#84
Only tests are missing, I will focus on them today and hopefully, all should be ready then.

@Kubuxu
Copy link
Member

Kubuxu commented Apr 29, 2019

We will have other things in the migration: ipfs/fs-repo-migrations#85

This was referenced Feb 28, 2020
Merged
Merged
@hsanjuan
Copy link
Contributor

hsanjuan commented Mar 5, 2020

Continuing work at #6955

Thanks @AuHau for this!

@hsanjuan hsanjuan closed this Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kubuxu Kubuxu Kubuxu approved these changes

Assignees
No one assigned
Labels
need/review Needs a review status/in-progress In progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AuHau @Stebalien @Kubuxu @hsanjuan