jsm: Replace listers with receive only channels #659
Conversation
| func (js *js) StreamsInfo() <-chan *StreamInfo { | ||
| ach := make(chan *StreamInfo) | ||
| sl := &streamLister{js: js} | ||
| go func() { |
There was a problem hiding this comment.
Feels like it should have timeouts and a way to cancel it etc no?
There was a problem hiding this comment.
sounds good, we could make it take options like, nats.MaxWait() and nats.Context() as I did with AckSync
There was a problem hiding this comment.
I tried adding options for both timeout and context (wallyqs@265ef17) but now thinking that might be simpler if only support context.Context instead for this case, so change to something like:
StreamsInfo(ctx context.Context) <-chan *StreamInfo
ConsumersInfo(ctx context.Context, stream string) <-chan *ConsumerInfo
test/js_test.go
Outdated
| t.Errorf("Unexpected error: %v", err) | ||
| var i int | ||
| expected := "foo" | ||
| for stream := range js.StreamsInfo() { |
There was a problem hiding this comment.
I like the way this flows imo.
jsm.go
Outdated
| @@ -24,41 +24,41 @@ import ( | |||
|
|
|||
| // JetStreamManager is the public interface for managing JetStream streams & consumers. | |||
| type JetStreamManager interface { | |||
There was a problem hiding this comment.
fyi - Java also has an accessor to get stream names.
There was a problem hiding this comment.
That is a good point, that API is not exposed in the JetStreamManager interface actually. Maybe we should add it too?
GetStreamBySubject(subj string) (*StreamInfo, error)https://github.com/nats-io/nats.go/blob/master/js.go#L655-L662
variadico
force-pushed
the
jsm-listers
branch
from
f2a2830
to
ffee0ba
Compare
variadico
force-pushed
the
jsm-listers
branch
4 times, most recently
from
d008667
to
ee948d9
Compare
wallyqs
changed the title
|
Need to have branch ready that replaces the lister APIs used by the server before merging. |
Signed-off-by: Waldemar Quevedo <wally@synadia.com>
|
Ready to update the server side after this PR merges. |
| var o jsmOpts | ||
| if len(opts) > 0 { | ||
| for _, opt := range opts { | ||
| if err := opt.configureJSManager(&o); err != nil { |
There was a problem hiding this comment.
maybe add a helper/DRY for some of these?
|
Closing in favor of this one #674 |
This adds the following two APIs to JSM which can be used instead of the listers.