check optional computation service when sending computation status#1018
check optional computation service when sending computation status#1018EtienneLt wants to merge 12 commits into
Conversation
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds a ChangesOptional Service Status Handling
Sequence Diagram(s)sequenceDiagram
participant Client
participant StudyService
participant RemoteServicesInspector
participant RootNetworkNodeInfoService
Client->>StudyService: getAllComputationsStatus()
StudyService->>RemoteServicesInspector: getOptionalServicesAsMap()
RemoteServicesInspector->>RemoteServicesInspector: isOptionalServiceUp(service) for each optional service
RemoteServicesInspector-->>StudyService: Map<String, ServiceStatusInfos>
StudyService->>StudyService: checkOptionalServiceIsUp(computationType)
alt service available and UP
StudyService->>RootNetworkNodeInfoService: query computation status
RootNetworkNodeInfoService-->>StudyService: status
else service unavailable
StudyService->>StudyService: omit computation type from result
end
StudyService-->>Client: computation statuses map
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/org/gridsuite/study/server/service/StudyService.java (1)
144-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare
remoteServicesInspectorasfinalfor consistency.Every other constructor-injected collaborator in
StudyServiceisprivate final; this new field breaks that convention and allows accidental reassignment.♻️ Proposed fix
- private RemoteServicesInspector remoteServicesInspector; + private final RemoteServicesInspector remoteServicesInspector;Also applies to: 209-210, 248-248
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/study/server/service/StudyService.java` at line 144, Declare the remoteServicesInspector field in StudyService as private final, matching the other constructor-injected collaborators, and ensure it is initialized through the constructor without reassignment.src/main/java/org/gridsuite/study/server/service/RemoteServicesInspector.java (1)
85-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate async aggregation logic between
getOptionalServices()andgetOptionalServicesAsMap().Both methods run the identical parallel health-check/join logic, differing only in the terminal collector. Simplify by having the map variant delegate to the list variant.
♻️ Proposed refactor
public Map<String, ServiceStatusInfos> getOptionalServicesAsMap() { - List<CompletableFuture<ServiceStatusInfos>> results = remoteServicesProperties.getServices().stream() - .filter(RemoteServicesProperties.Service::isOptional) - .map(service -> asyncSelf.isOptionalServiceUp(service).thenApply(isUp -> ServiceStatusInfos.builder() - .name(service.getName()) - .status(Boolean.TRUE.equals(isUp) ? ServiceStatus.UP : ServiceStatus.DOWN) - .build())) - .toList(); - CompletableFuture.allOf(results.toArray(CompletableFuture[]::new)).join(); - return results.stream().map(CompletableFuture::join).collect(Collectors.toMap(ServiceStatusInfos::name, Function.identity())); + return getOptionalServices().stream().collect(Collectors.toMap(ServiceStatusInfos::name, Function.identity())); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/study/server/service/RemoteServicesInspector.java` around lines 85 - 109, Remove the duplicated asynchronous health-check pipeline from getOptionalServicesAsMap() by delegating to getOptionalServices() and collecting its returned ServiceStatusInfos with Collectors.toMap(ServiceStatusInfos::name, Function.identity()). Preserve the existing optional-service filtering, status construction, and map key behavior through the list method.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/gridsuite/study/server/service/StudyService.java`:
- Around line 3715-3720: The getAllComputationsStatus method triggers uncached
optional-service health checks on every request. Add a short-TTL cache for the
result of RemoteServicesInspector.getOptionalServicesAsMap(), ensuring
concurrent requests reuse an in-flight or recently completed lookup while
retaining refresh behavior after expiration; update getAllComputationsStatus to
use this cached path.
- Around line 3726-3728: The dynamic margin calculation status is guarded by the
wrong optional-service constant in the surrounding status-building logic. In the
`allComputationStatus` block, replace `DYNAMIC_MAPPING_SERVER` with the service
key/constant corresponding to `DYNAMIC_MARGIN_CALCULATION` when calling
`checkOptionalServiceIsUp`.
---
Nitpick comments:
In
`@src/main/java/org/gridsuite/study/server/service/RemoteServicesInspector.java`:
- Around line 85-109: Remove the duplicated asynchronous health-check pipeline
from getOptionalServicesAsMap() by delegating to getOptionalServices() and
collecting its returned ServiceStatusInfos with
Collectors.toMap(ServiceStatusInfos::name, Function.identity()). Preserve the
existing optional-service filtering, status construction, and map key behavior
through the list method.
In `@src/main/java/org/gridsuite/study/server/service/StudyService.java`:
- Line 144: Declare the remoteServicesInspector field in StudyService as private
final, matching the other constructor-injected collaborators, and ensure it is
initialized through the constructor without reassignment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d27a72fe-4de4-4771-8c3b-1a3434dc6e3c
📒 Files selected for processing (4)
src/main/java/org/gridsuite/study/server/service/RemoteServicesInspector.javasrc/main/java/org/gridsuite/study/server/service/RootNetworkNodeInfoService.javasrc/main/java/org/gridsuite/study/server/service/StudyService.javasrc/main/resources/application-local.yml
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
Signed-off-by: Etienne LESOT <etienne.lesot@rte-france.com>
|
1 participant
PR Summary