[notify] Introduce StreamAllTransactions stream#620
Conversation
|
coverage: 91.5% (-0.001%) from 91.501% — liran-funaro:notify-full-block into hyperledger:main |
liran-funaro
force-pushed
the
notify-full-block
branch
5 times, most recently
from
c960635 to
f26a171
Compare
Signed-off-by: Liran Funaro <liran.funaro@gmail.com>
liran-funaro
force-pushed
the
notify-full-block
branch
from
f26a171 to
8f0927f
Compare
cendhu
left a comment
cendhu
left a comment
There was a problem hiding this comment.
LGTM. In a separate PR, we should also update our loadgen to use this stream to avoid processing blocks.
| return events | ||
| } | ||
|
|
||
| func getMatchingNamespaces( |
There was a problem hiding this comment.
getMatching and filterNamespaces are confusing.Let's use filter everywhere as it denotes what we let in as per our context. My suggestion would be applyFilter or something similar.
There was a problem hiding this comment.
I used filterAndBuildEvents() and filterNamespaces()
| n.metrics.notifierActiveStreams.Inc() | ||
| defer n.metrics.notifierActiveStreams.Dec() |
There was a problem hiding this comment.
I assume the MaxConcurrentStream and stream interceptor are applicable for this stream too automatically.
There was a problem hiding this comment.
Yes. It is tested in the integration test.
Signed-off-by: Liran Funaro <liran.funaro@gmail.com>
|
@cendhu There is a problem with the linter download. I ran the linter locally and it passes. |
in preparation for hyperledger/fabric-x-committer#620. After that is merged, we can add an integration test to the SDK. Signed-off-by: Arne Rutjes <arne123@gmail.com>
3 participants
Type of change
Description
Related issues