Skip to content

Conversation

AhmedSoliman
Copy link
Contributor

@AhmedSoliman AhmedSoliman commented May 1, 2025

Copy link

github-actions bot commented May 1, 2025

Test Results

  7 files  ±0    7 suites  ±0   4m 52s ⏱️ -7s
 54 tests ±0   53 ✅ ±0  1 💤 ±0  0 ❌ ±0 
223 runs  ±0  220 ✅ ±0  3 💤 ±0  0 ❌ ±0 

Results for commit a4a1a88. ± Comparison against base commit 0fe0c42.

♻️ This comment has been updated with latest results.

@AhmedSoliman AhmedSoliman force-pushed the pr3215 branch 2 times, most recently from 6aafcc4 to 1dca721 Compare May 1, 2025 10:28
@AhmedSoliman AhmedSoliman requested a review from tillrohrmann May 1, 2025 15:46
@AhmedSoliman AhmedSoliman force-pushed the pr3215 branch 3 times, most recently from 2870202 to 2a5c795 Compare May 2, 2025 10:50
Copy link
Contributor

@tillrohrmann tillrohrmann left a comment

Choose a reason for hiding this comment

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

Great work @AhmedSoliman :-) LGTM. +1 for merging.

Comment on lines 384 to 391
Err(
// We don't know this node-id yet, but we might in the future, keep retrying.
err @ ConnectError::Discovery(DiscoveryError::UnknownNodeId(_)),
) => {
trace!(%swimlane, %peer, "Cannot to connect, will retry: {}", err);
continue 'connect;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be a busy loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch. I'll add a hardcoded 500ms sleep here.

Other changes:
- Smaller EgressMessages by using static string for message names, removing span for unary messages (224 bytes -> 216 bytes)
- ReplyRx doesn't require protocol version at creation time, this will become relevant in a subsequent PR. The protocol version is now returned along with the reply when it arrives
@AhmedSoliman AhmedSoliman merged commit a4a1a88 into main May 6, 2025
51 checks passed
@AhmedSoliman AhmedSoliman deleted the pr3215 branch May 6, 2025 11:02
@github-actions github-actions bot locked and limited conversation to collaborators May 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants