feat: 2797 rsa bootstrap followup#2862
Open
berryware wants to merge 8 commits into
Open
Conversation
… has been put into the pbj repo for a fix ____________________________________________________________________ Signed-off-by: berryware <[email protected]> # Conflicts: # block-node/roster-bootstrap-rsa/build.gradle.kts
- remove gson from the plugin ____________________________________________________________________ Signed-off-by: berryware <[email protected]>
…Node a 17 minute window to start up. ____________________________________________________________________ Signed-off-by: berryware <[email protected]>
…usly in the background - add an executor to run the background task. - cancel the task once an addressbook is found - update tests to pass - remove the test for IllegalStateException ____________________________________________________________________ Signed-off-by: berryware <[email protected]>
____________________________________________________________________ Signed-off-by: berryware <[email protected]>
berryware
requested review from
AlfredoG87,
Nana-EC,
ata-nas,
ivannov and
jsync-swirlds
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #2862 +/- ##
============================================
+ Coverage 81.47% 81.61% +0.13%
- Complexity 1559 1562 +3
============================================
Files 144 144
Lines 7413 7412 -1
Branches 777 777
============================================
+ Hits 6040 6049 +9
+ Misses 1050 1039 -11
- Partials 323 324 +1
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Contributor
jsync-swirlds
left a comment
jsync-swirlds
left a comment
There was a problem hiding this comment.
A few minor items (which are not required) and a couple questions.
…node/roster/bootstrap/rsa/RsaRosterBootstrapConfig.java Co-authored-by: Joseph S. <[email protected]> Signed-off-by: David Berry <[email protected]>
…node/roster/bootstrap/rsa/RsaRosterBootstrapPlugin.java Co-authored-by: Joseph S. <[email protected]> Signed-off-by: David Berry <[email protected]>
…node/roster/bootstrap/rsa/RsaRosterBootstrapPlugin.java Co-authored-by: Joseph S. <[email protected]> Signed-off-by: David Berry <[email protected]>
Nana-EC
reviewed
May 21, 2026
| NodeAddressBook.newBuilder().nodeAddress(addresses).build(); | ||
|
|
||
| // We found an address book stop the mirror node requests | ||
| scheduledFuture.cancel(true); |
Contributor
There was a problem hiding this comment.
nit: stop the schedule after a successful updateAddressBook call, to ensure data goes out first
Comment on lines
+231
to
+233
| "RSA roster available: {0} entries obtained from {1}", | ||
| rosterEntriesLoaded, | ||
| addressBookSource, |
Contributor
There was a problem hiding this comment.
Suggested change
| "RSA roster available: {0} entries obtained from {1}", | |
| rosterEntriesLoaded, | |
| addressBookSource, | |
| "RSA roster available: {0} entries obtained from {1}", | |
| book.nodeAddress().size(), | |
| "Mirror Node", |
|
|
||
| start(plugin, new SimpleInMemoryHistoricalBlockFacility(), serverConfig()); | ||
|
|
||
| latch.await(); |
Contributor
There was a problem hiding this comment.
This (and others) should have a timeout
| /// UncaughtExceptionHandler for logging uncaught exceptions | ||
| private void uncaughtExceptionHandler(Thread thread, Throwable throwable) { | ||
| LOGGER.log( | ||
| WARNING, "Uncaught exception in RsaRosterBootstrapPlugin thread {0}: {1}", thread.getName(), throwable); |
Contributor
There was a problem hiding this comment.
I think the throwable needs to be the trailing argument after all substitutions right?
Suggested change
| WARNING, "Uncaught exception in RsaRosterBootstrapPlugin thread {0}: {1}", thread.getName(), throwable); | |
| WARNING, "Uncaught exception in RsaRosterBootstrapPlugin thread {0}", thread.getName(), throwable); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reviewer Notes
Submitting these two seperately as a partial completion of #2797 as these are higher priority
Related Issue(s)