-
Notifications
You must be signed in to change notification settings - Fork 895
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
update some deprecated references #8314
update some deprecated references #8314
Conversation
Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: Sally MacFarlane <[email protected]>
@@ -100,7 +100,7 @@ public Optional<byte[]> get(final SegmentIdentifier segmentId, final byte[] key) | |||
throwIfClosed(); | |||
|
|||
try (final OperationTimer.TimingContext ignored = metrics.getReadLatency().startTimer()) { | |||
return Optional.ofNullable(snapTx.get(columnFamilyMapper.apply(segmentId), readOptions, key)); | |||
return Optional.ofNullable(snapTx.get(readOptions, columnFamilyMapper.apply(segmentId), key)); |
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.
parameter 1 and 2 swapped
@@ -524,7 +525,7 @@ MessageData constructGetTrieNodesResponse(final MessageData message) { | |||
var trieNode = optStorage.orElse(Bytes.EMPTY); | |||
if (!trieNodes.isEmpty() | |||
&& (sumListBytes(trieNodes) + trieNode.size() > maxResponseBytes | |||
|| stopWatch.getTime() | |||
|| stopWatch.getTime(TimeUnit.MILLISECONDS) |
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.
time units passed to method. I think milliseconds is right?
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.
yeah milliseconds is the right unit
@@ -41,11 +41,12 @@ public void readGenesisFromObjectNode() { | |||
final var configNode = mapper.createObjectNode(); | |||
configNode.put("londonBlock", 1); | |||
final var allocNode = mapper.createObjectNode(); | |||
allocNode.put(Address.BLS12_G2MULTIEXP.toUnprefixedHexString(), generateAllocation(Wei.ONE)); | |||
allocNode.putIfAbsent( |
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.
Why the change from put to putIfAbsent? I guess either should be fine since the object node is being created in the test
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.
put with those args (String, JsonNode) is deprecated. putIfAbsent was an easy fix and yeah - it's a test, and the test still passes, and I didn't think any further about it
@@ -524,7 +525,7 @@ MessageData constructGetTrieNodesResponse(final MessageData message) { | |||
var trieNode = optStorage.orElse(Bytes.EMPTY); | |||
if (!trieNodes.isEmpty() | |||
&& (sumListBytes(trieNodes) + trieNode.size() > maxResponseBytes | |||
|| stopWatch.getTime() | |||
|| stopWatch.getTime(TimeUnit.MILLISECONDS) |
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.
yeah milliseconds is the right unit
PR description
Update some code to eliminate some deprecated method calls.
The RocksDB one is just a method param order swap, looks benign.
Fixed Issue(s)
Thanks for sending a pull request! Have you done the following?
doc-change-required
label to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew spotlessApply
./gradlew build
./gradlew acceptanceTest
./gradlew integrationTest
./gradlew ethereum:referenceTests:referenceTests