Upgrade to Java 21 #995
Upgrade to Java 21 #995
Conversation
-- gatlingImplementation is commented.
There was a problem hiding this comment.
We need give some notice about the breaking change of requiring Java 21 before merging this in. So for this upcoming release we can place a notice in our changelog and the announcement about the upcoming change and next monthly release change to using Java 21.
# Conflicts: # gradle/wrapper/gradle-wrapper.properties
usmansaleem
added
the
doc-change-required
| } | ||
|
|
||
| task downloadBesu(type: Download) { | ||
| src { | ||
| // see gradle.properties for Besu URL | ||
| return new SimpleTemplateEngine().createTemplate(besuDistroUrl).make(["besuVersion":"$besuVersion"]).toString() | ||
| } | ||
| dest new File(buildDir, "besu-${besuVersion}.tar.gz") | ||
| dest layout.buildDirectory.file("downloads/besu-${besuVersion}.tar.gz") |
There was a problem hiding this comment.
Does the downloads directory get created if doesn't exist? And will work on Windows with the hardcoded / directory separator?
There was a problem hiding this comment.
This works on nix without any problems. Need to check our whole build on Windows environment.
| requestUri = failureContext.request().absoluteURI(); | ||
| } catch (final NullPointerException e) { | ||
| // absoluteURI can throw an NPE if host or port is missing | ||
| requestUri = "Error in calculating request URI"; |
There was a problem hiding this comment.
More descriptive error would be nicer here along line of invalid URI due to missing host or port.
There was a problem hiding this comment.
sure. This NPE happens when invalid host header is involved from the client. Latest vertx has started throwing an NPE. I'll update the error description.
There was a problem hiding this comment.
refactored it a bit.
slashing-protection/src/main/java/tech/pegasys/web3signer/slashingprotection/DbLocker.java
Outdated
Show resolved
Hide resolved
| final Response response = | ||
| given().baseUri(getMetricsUrl()).contentType(ContentType.JSON).when().get(METRICS_ENDPOINT); | ||
|
|
||
| final List<String> lines = | ||
| Arrays.asList(response.getBody().asString().split(String.format("%n")).clone()); | ||
| return convertToLines(response.getBody().asString()).stream() |
There was a problem hiding this comment.
This could be simplified to:
response.getBody().asString().lines()
Removing the need for convertToLines
There was a problem hiding this comment.
nice suggestion, updated.
| @@ -204,16 +204,29 @@ public static String signPath(final KeyType keyType) { | |||
| return keyType == BLS ? ETH2_SIGN_ENDPOINT : ETH1_SIGN_ENDPOINT; | |||
| } | |||
|
|
|||
| public Set<String> getMetricsMatching(final List<String> metricsOfInterest) { | |||
| public Map<String, String> getMatchedMetrics(final Set<String> metricsOfInterest) { | |||
There was a problem hiding this comment.
nit: IMHO the previous name was clearer
build.gradle
Outdated
| } | ||
| removeUnusedImports() | ||
| // removeUnusedImports() // conflicts with googleJavaFormat for 7.0.0.BETA1 |
| // Let the next matching route or error handler deal with the error, we only handle logging | ||
| failureContext.next(); | ||
| } else { | ||
| LOG.warn("Error handler triggered without any propagated failure"); | ||
| } | ||
| } | ||
|
|
||
| private static String getRequestUri(RoutingContext failureContext) { |
There was a problem hiding this comment.
I think the method name isn't quite right as the error case it doesn't return a requestUri. Would be better to return an empty and just include that error message in the handle method.
There was a problem hiding this comment.
agreed. Updated the method to return Optional.
| LOG.error( | ||
| String.format("Failed request: %s", failureContext.request().absoluteURI()), | ||
| failureContext.failure()); | ||
| final String requestUri = |
There was a problem hiding this comment.
Not going to block on this, but the variable name is a bit misleading as sometimes it is an error message and sometimes it's the requestUri.
joaniefromtheblock
removed
the
doc-change-required
PR Description
Upgrade build and runtime to Java 21. Following changes are also made to achieve this:
Logs after upgrading to Java 21:
Fixed Issue(s)
fixes #1013
Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog
Testing