🐛 using local flag instead of setting client to nil #87
Conversation
|
/assign @qiujian16 |
skeeey
force-pushed
the
flag
branch
2 times, most recently
from
2148910
to
f23bfe7
Compare
| for { | ||
| if c.currentClient() == nil { | ||
| if retrying { |
There was a problem hiding this comment.
maybe have a clientReady() bool func, and check here?
There was a problem hiding this comment.
I think it may be not necessary, because the retrying flag always set after the client closed
There was a problem hiding this comment.
using client ready instead of retrying flag
|
|
||
| for { | ||
| if cloudEventsClient != nil { | ||
| if startReceiving { |
There was a problem hiding this comment.
why not use clientReady here also, the subscribe/send may change at the same time?
There was a problem hiding this comment.
we always start the subscribe after the client connected, this is controlled by restartReceiverSignal channel in the base client internal, so I did not use the clientReady here
but for send part, it will be called by outside, so when we send an event, we need check the client if ready or not
| if cloudEventsClient == nil { | ||
| c.RLock() | ||
| defer c.RUnlock() | ||
| if !c.clientReady { |
There was a problem hiding this comment.
use c.isClientReady() here also
Signed-off-by: Wei Liu <[email protected]>
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiujian16, skeeey The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit f50d6e8
into
open-cluster-management-io:main
Summary
Currently, we set the client to nil when a client start reconnecting, this may cause nil point
In this pr, we use local flag to mark the client not ready