-
Notifications
You must be signed in to change notification settings - Fork 972
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
feat(blob): blobsub #3539
feat(blob): blobsub #3539
Conversation
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.
Lovely
Do we want to block this PR on absence proofs? The rough consensus from our call is that we should. Thoughts? |
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.
lint.
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.
Agree with hlib's comments re buffer + re shutdown
Co-authored-by: Hlib Kanunnikov <[email protected]>
Co-authored-by: Hlib Kanunnikov <[email protected]>
Introduces
blob.Subscribe
. Takes ashare.Namespace
, returns a<-chan *BlobsubResponse
containing aheight uint64
andblobs []*blob.Blob
.I made the decision to not support subscribing to multiple namespaces with one call, because the complexity of the return type mitigates the benefits for the client. It is likely more convenient to have one subscription per namespace:
Imagine you are watching two namespaces for different things. On the client side, you would have a
select
statement reading from the two channels in two cases. This is cleaner than receiving everything in one case, but then having to manually parse and send to the respective handlers yourself.This closes #2737 , with the caveat that we are not taking the map approach (see rationale above)