-
Notifications
You must be signed in to change notification settings - Fork 6
fix: Fix BlockAccessService - getBlock - when using only retrieve_latest=true
#1146
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
Conversation
retrieve_latest=true
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.
Looking good but one suggestion to include more coverage
...block-access/src/main/java/org/hiero/block/node/access/service/BlockAccessServicePlugin.java
Show resolved
Hide resolved
...block-access/src/main/java/org/hiero/block/node/access/service/BlockAccessServicePlugin.java
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.
LG to unblock.
The test case for -1 should probably be restored later but with a different error condition
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.
LG
...k-access/src/test/java/org/hiero/block/node/access/service/BlockAccessServicePluginTest.java
Outdated
Show resolved
Hide resolved
...k-access/src/test/java/org/hiero/block/node/access/service/BlockAccessServicePluginTest.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.
LG
This PR should not apply to main, as it creates a bug to work around a bad assumption. |
...block-access/src/main/java/org/hiero/block/node/access/service/BlockAccessServicePlugin.java
Show resolved
Hide resolved
…latest` set. Signed-off-by: Alfredo Gutierrez Grajeda <alfredo@hashgraph.com>
6d57fab
to
c0af20c
Compare
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.
Appropriate as work around for v0.10.0 testing
superseeded by #1147 and founded another workaround so no need to rush this change into current release. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## release/0.10 #1146 +/- ##
==================================================
- Coverage 84.86% 84.84% -0.02%
+ Complexity 916 914 -2
==================================================
Files 104 104
Lines 3719 3715 -4
Branches 396 395 -1
==================================================
- Hits 3156 3152 -4
Misses 411 411
Partials 152 152
🚀 New features to boost your workflow:
|
Reviewer Notes
current request does not work:
with this temporary fix, we make it work.
but it could also work when: