-
Notifications
You must be signed in to change notification settings - Fork 602
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
NIP96 - Adding pubkeys to file urls to allow data migration between media servers. #1097
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Not a fan of changing the URL scheme now. Also, I do not see the point of this or how it will help anything. There might be a use case for this, but since we do not (or should not) allow directory browsing, having npub in the URL does not really help.
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.
Yes, especially when #1236 would allow the user to list their files and easily migrate them.
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.
The goal is to make it easy to know if a file was uploaded by a pubkey or another, it is clear that with a call to the API we can have it, but for a user it will not be easy.
I think this is in line with what blossom promotes, url's standards, in this case the original pubkey (in my opinion) is very relevant.
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 am not sure if you are considering that maintaining additional information comes at a cost, compute or otherwise. Adding npub to a URL will require to somehow know that this npub has this hash under it, and that some other npub might have the same hash. The less info you need to resolve what to serve, the cheaper, faster and more efficient you can do it.
I see it all the time on the nostr, most people think in single servers and single DBs, and nothing about how and when can you scale. If you run a single server then you are good. If you run a highly distributed infra across the globe and trying to shave nanoseconds of time from each request because you serve thousands a second, and trying to remove any possible call to anything when serving a file. Then you are screwed with all of this. It is nice to have things that nobody needs or cares about.
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.
@quentintaranpino I think a better and simpler way to solve the issue you mention on this pull request is the following:
This does not require having the npub in the url.
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.
The goal was to not rely on API calls to know if an image was uploaded by a pubkey. In case nobody sees the point, I give up 😂
@arthurfranca @v0l @fishcakeday @sant0s12
I am going to close this PR, if you think there is something interesting in it you can add it to the other PR's that are open.
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.
The spirit of this PR will live on
https://link.to/<image-sha256>.png#ox=<hash>&nip96u=<pubkey>
kind of urls if nip96u tag is added heheh