-
Notifications
You must be signed in to change notification settings - Fork 518
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 3326 : k8s resource acknowledge the port in application.properties for Micronaut application #3366
Fix 3326 : k8s resource acknowledge the port in application.properties for Micronaut application #3366
Conversation
…plication.properties, fix eclipse-jkube#3326 Signed-off-by: arman-yekkehkhani <arman.yekkehkhani@gmail.com>
…clipse-jkube#3326 Signed-off-by: arman-yekkehkhani <arman.yekkehkhani@gmail.com>
…se-jkube#3326 Signed-off-by: arman-yekkehkhani <arman.yekkehkhani@gmail.com>
Eclipse JKube CI ReportStarted new GH workflow run for #3366 (2024-10-13T04:08:07Z) ⚙️ JKube E2E Tests (11311214381)
|
…e-jkube#3326 Signed-off-by: arman-yekkehkhani <arman.yekkehkhani@gmail.com>
.map(ImageConfiguration::getBuild).map(BuildConfiguration::getPorts) | ||
.orElse(Collections.emptyList()).stream() | ||
.findFirst().orElse(null); | ||
|
||
if (port == null) { | ||
port = extractPort(getMicronautConfiguration(getContext().getProject()), null); |
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.
Do you think we can move this to orElse
method of the above statement?
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! applied the changes.
} | ||
java { | ||
sourceCompatibility = JavaVersion.toVersion("17") | ||
targetCompatibility = JavaVersion.toVersion("17") |
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.
I think you've added this because of Micronaut 4 JDK 17 baseline requirement. This will fail on JDK 11 . Not sure whether we should move this test to micronaut 3
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.
Should we roll back to Micronaut 3 for the time being? Is there any hard limit on the JDK version for IT?
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.
I'll check with Marc about this and get back to you.
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.
Let's keep this Java11 friendly so we can merge it.
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.
@arman-yekkehkhani : Could you please use micronaut 3 instead?
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.
@rohanKanojia @manusa Thanks! changed Micronaut 4 to 3.
@@ -85,10 +85,11 @@ private Probe buildProbe(Integer initialDelaySeconds, Integer periodSeconds){ | |||
return null; | |||
} | |||
|
|||
final String firstImagePort = getImages().stream().findFirst() | |||
String port = getImages().stream().findFirst() |
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.
Could you please revert it to the previous name? I think it was more accurate than the renamed version.
String port = getImages().stream().findFirst() | |
final String firstImagePort = getImages().stream().findFirst() |
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.
@rohanKanojia Yeah, thanks. applied the changes.
…ipse-jkube#3326 Signed-off-by: arman-yekkehkhani <arman.yekkehkhani@gmail.com>
*/ | ||
plugins { | ||
id("com.github.johnrengelman.shadow") version "7.1.0" | ||
id("io.micronaut.application") version "3.0.2" |
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.
Could you please use the latest version of 3.x release that's compatible with JDK11 ?
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.
Sure. updated.
fix eclipse-jkube#3326 Signed-off-by: arman-yekkehkhani <arman.yekkehkhani@gmail.com>
@arman-yekkehkhani : Could you please resolve minor conflicts? I think this is good to be merged. |
# Conflicts: # CHANGELOG.md
@rohanKanojia Sure! It should be fixed now. |
Quality Gate passedIssues Measures |
# Conflicts: # CHANGELOG.md
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, thx!
Description
Fixes #3326
Type of change
test, version modification, documentation, etc.)
Checklist