davsclaus commented on PR #24663:
URL: https://github.com/apache/camel/pull/24663#issuecomment-4959329775
Thanks for the review @gnodet! All three inline findings have been addressed
in 17b46ab:
1. **MBeanServer** — switched to string-based `MBeanServer.invoke()` via
`ObjectName("com.sun.management:type=HotSpotDiagnostic")`, matching the
`HeapHistogramDevConsole` pattern.
2. **Path sanitization** — added `Path.of(name).getFileName().toString()` to
strip path separators.
3. **Code duplication** — extracted shared `TuiHelper.triggerHeapDump()`
used by both `MemoryTab` and `MemoryLeakTab`.
On the remaining items:
4. **Tests** — agreed, the dev console modules in `camel-console` currently
have no test infrastructure set up. The heap dump action requires a running JVM
with the HotSpot MBean, which makes unit testing non-trivial (the `dumpHeap`
call creates a real file). We can add basic tests for the filename sanitization
and default-name logic in a follow-up.
5. **MCP pattern** — correct, the heap dump tool uses `executeAction()`
directly because it needs to pass the `name` and `live` parameters through the
action file protocol, which doesn't fit the `delegateToRegistry()` pattern used
for simple get-style calls like heap histogram.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
