-
Notifications
You must be signed in to change notification settings - Fork 3
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
Blobs #5
Merged
Merged
Blobs #5
Conversation
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
The code recognizes blobs in two message content types: about and post. For about messages the code checks the "image" field. This field can be a string containing a blob ref or a mention object. For post messages the code checks the mentions field. This field is a list of mention objects.
One thing that comes to mind is storing another reason for keeping a blob e.g. explicit pin requests made by the UI. Another thing could be caching the size of a blob.
Blobs are sent in chunks to avoid using up too much wire time in one handler.
This implements replying to "wants" messages with "has" messages. "Wants" messages are received through `blobs.createWants` stream initiated by the local node and have to trigger a reply with a "has" message through `blobs.createWants` stream initiated by the remote node.
After this change the Swift code can request blobs to be downloaded and the blobs remain in the want list for a specific amount of time. This replace the previous approach of downloading all blobs right away and copies the current behaviour exhibited by go-ssb. The new code behaves slightly better as the want list isn't cleared when application exists and is persisted.
I think it makes sense to either try to review this again and merge it or just merge it right away. At this point this functionality more-or-less works, we are not in production and this branch is basically blocking merges to |
Ok, after a short discussion let's just merge this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Related to #1.