Skip to content

Add bg WS connect to common utility - superseding initialConnectAttempts#183

Merged
EnriqueL8 merged 6 commits intomainfrom
bg-connect
May 22, 2025
Merged

Add bg WS connect to common utility - superseding initialConnectAttempts#183
EnriqueL8 merged 6 commits intomainfrom
bg-connect

Conversation

@peterbroadhurst
Copy link
Contributor

The journey (multiple hops) that lead to hyperledger/firefly#1315 told us that initialConnectAttempts was a bit of a flawed coupling strategy between asynchronously starting microservices.

We abandoned it, but left the feature in the common library without a clue that it was flawed.

So this PR tries to help other users like me from repeating the mistake :)

…ConnectAttempts

Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
@peterbroadhurst peterbroadhurst requested a review from a team as a code owner May 22, 2025 03:00
peterbroadhurst and others added 2 commits May 21, 2025 23:10
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: John Hosie <john.hosie@kaleido.io>
Copy link
Contributor

@EnriqueL8 EnriqueL8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - but don't understand the config change, feels like a missing piece there on how that interface is used

include BackgroundConnect in GenerateConfig
@EnriqueL8
Copy link
Contributor

Merged a fix from @hosie and test coverage as well

Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Copy link
Contributor

@EnriqueL8 EnriqueL8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@EnriqueL8 EnriqueL8 merged commit 599b7b1 into main May 22, 2025
5 checks passed
@EnriqueL8 EnriqueL8 deleted the bg-connect branch May 22, 2025 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants