-
Notifications
You must be signed in to change notification settings - Fork 143
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
fix: app and services to stop querying the platform for the roster #16771
base: main
Are you sure you want to change the base?
fix: app and services to stop querying the platform for the roster #16771
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #16771 +/- ##
=============================================
- Coverage 64.28% 64.27% -0.01%
+ Complexity 20886 20884 -2
=============================================
Files 2549 2549
Lines 95969 95971 +2
Branches 10041 10042 +1
=============================================
- Hits 61690 61689 -1
- Misses 30653 30655 +2
- Partials 3626 3627 +1
|
Signed-off-by: Miroslav Gatsanoga <[email protected]>
0a177a5
to
20a5ea8
Compare
Signed-off-by: Neeharika-Sompalli <[email protected]>
Signed-off-by: Miroslav Gatsanoga <[email protected]>
… initializing states api Signed-off-by: Miroslav Gatsanoga <[email protected]>
Signed-off-by: Neeharika-Sompalli <[email protected]>
Signed-off-by: Neeharika-Sompalli <[email protected]>
Signed-off-by: Neeharika-Sompalli <[email protected]>
Signed-off-by: Neeharika-Sompalli <[email protected]>
Signed-off-by: Neeharika-Sompalli <[email protected]>
Signed-off-by: Neeharika-Sompalli <[email protected]>
Signed-off-by: Neeharika-Sompalli <[email protected]>
Signed-off-by: Neeharika-Sompalli <[email protected]>
Signed-off-by: Neeharika-Sompalli <[email protected]>
Co-authored-by: Joseph S. <[email protected]> Signed-off-by: Neeharika Sompalli <[email protected]>
Co-authored-by: Joseph S. <[email protected]> Signed-off-by: Neeharika Sompalli <[email protected]>
Signed-off-by: Miroslav Gatsanoga <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Miroslav Gatsanoga <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
...ents/src/main/java/com/hedera/services/bdd/junit/hedera/embedded/AbstractEmbeddedHedera.java
Outdated
Show resolved
Hide resolved
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.
LGTM! Thanks @MiroslavGatsanoga
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
...ents/src/main/java/com/hedera/services/bdd/junit/hedera/embedded/AbstractEmbeddedHedera.java
Outdated
Show resolved
Hide resolved
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.
Thanks @MiroslavGatsanoga!
Replacing rosterEntries.size()
with networkInfo.addressBook().size()
is definitely correct, and this PR doesn't need to update the canAdopt
predicate.
But we don't want to go back to out-of-band RosterService
state modifications in AbstractEmbeddedHedera
; and in fact that should not be necessary.
Can you merge branch 17056-roster-lifecycle-pr-checks
and verify that the AbstractEmbeddedHedera
diff in this PR can be removed?
…top-querying-platform-for-roster
Signed-off-by: Miroslav Gatsanoga <[email protected]>
Signed-off-by: Miroslav Gatsanoga <[email protected]>
47d3275
Signed-off-by: Miroslav Gatsanoga <[email protected]>
Signed-off-by: Miroslav Gatsanoga <[email protected]>
@@ -1004,8 +1007,13 @@ private void initializeDagger(@NonNull final State state, @NonNull final InitTri | |||
.round(); | |||
final var initialStateHash = new InitialStateHash(initialStateHashFuture, roundNum); | |||
|
|||
final var activeRoster = tssBaseService.chooseRosterForNetwork( | |||
state, trigger, serviceMigrator, version, configProvider.getConfiguration(), platform.getRoster()); | |||
Roster activeRoster; |
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.
addressBook.useRosterLifecycle=true
.
But in production environments up to 0.58, there will be no active roster, and this will lead to a NPE when constructing StateNetworkInfo
below.
We may want to delay this PR until we have been able to make useRosterLifecycle=true
the default.
Description:
Remove
Platform.getRoster()
calls from Services code.Related issue(s):
Fixes #16721
Checklist