Skip to content

Conversation

@Yavor16
Copy link
Contributor

@Yavor16 Yavor16 commented Aug 21, 2025

No description provided.

@Yavor16 Yavor16 force-pushed the add-readiness-healthcheck branch from c459828 to d50f79c Compare August 21, 2025 13:31
@Yavor16 Yavor16 changed the title Add routable implementation Add health checks again Aug 22, 2025
.type("web")
.uptime(9042L)
.fileDescriptorQuota(1024L)
.routable(routable)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why value is not boolean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because in the ProcessStatisticsResource class the routable variable is String

theghost5800
theghost5800 previously approved these changes Aug 26, 2025
Copy link
Contributor

@theghost5800 theghost5800 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Squash before merge

Comment on lines +120 to +123
private boolean isThereAtLeastOneRoutedInstance(List<InstanceInfo> instanceInfos) {
return instanceInfos.stream()
.anyMatch(InstanceInfo::isRoutable);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will be checking if there is at least one routeable instance? Compared to current approach with the started state, we wait for all instances. This is inconsistency, is it deliberately this way?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if there is defined readiness health check type, we will wait for one instance to be routable instead of all of them. If there isn't readiness health check type defined, we will work like we do now

Integer readinessHealthCheckInvocationTimeout = null;
String readinessHealthCheckHttpEndpoint = null;
Integer readinessHealthCheckInterval = null;
if (readinessHealthCheckType.getData() != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can readinessHealthCheckType be null and throw npe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the type can't be null because it has default value. In the documentation is said that the default value is process

LMCROSSITXSADEPLOY-3140
@Yavor16 Yavor16 force-pushed the add-readiness-healthcheck branch from fcf09bc to 4a9c779 Compare August 28, 2025 10:20
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
46.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@Yavor16 Yavor16 merged commit 7e18e2b into master Aug 28, 2025
7 of 8 checks passed
@Yavor16 Yavor16 deleted the add-readiness-healthcheck branch August 28, 2025 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants