-
Notifications
You must be signed in to change notification settings - Fork 37
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
streamr-1.0 fix-start-and-stop-of-autocertifier-client #2182
base: main
Are you sure you want to change the base?
Conversation
@@ -36,7 +36,6 @@ export interface IAutoCertifierClient { | |||
stop(): void | |||
on(eventName: string, cb: (subdomain: CertifiedSubdomain) => void): void | |||
} | |||
|
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.
Keep this line
const requests = Array.from(this.ongoingConnectRequests.values()) | ||
await Promise.allSettled(requests.map((conn) => conn.close(false))) | ||
if (this.autoCertifierClient) { | ||
await this.autoCertifierClient.stop() |
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.
await this.autoCertifierClient.stop() | |
this.autoCertifierClient.stop() |
// all paths of this.autoCertifierClient.start() will block until 'updatedCertificate' | ||
// event is emitted or an exception is thrown | ||
|
||
await withTimeout(this.autoCertifierClient.start(), START_TIMEOUT, |
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.
Maybe we should separate the starting of the client and obtaining the initial certificate. It doesn't take long time to start the client, but in practice we wait for the certificate?
Also could rename AutoCertifierClientFacade
to AutoCertifierFacade
as we don't seem to use any of the client's functionality methods (we just call the start
and stop
methods).
-> maybe in a separate PR
This is the fix-start-and-stop-of-autocertifier-client pull request moved to base upon streamr-1.0 instead of testnet-one