Copilot commented on code in PR #2164:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2164#discussion_r3159664784
##########
extension-framework/cpp-extension-lib/include/api/core/Resource.h:
##########
@@ -103,13 +103,10 @@ void useProcessorClassDescription(Fn&& fn) {
.destroy = [] (MINIFI_OWNED void* self) -> void {
delete static_cast<Class*>(self);
},
Review Comment:
`MinifiProcessorCallbacks` no longer exposes an `isWorkAvailable` callback,
but the C++ extension base (`api::core::ProcessorImpl`) still provides a
virtual `isWorkAvailable()` hook that extension authors may override expecting
it to affect scheduling. With this mapping removed, such overrides will never
be invoked, which is likely to cause confusing/“dead” API surface for extension
developers.
Consider either (1) removing/deprecating `ProcessorImpl::isWorkAvailable()`
in the cpp-extension-lib API, or (2) adding a clear note in the extension API
docs that `isWorkAvailable` is no longer consulted and that
`getTriggerWhenEmpty` + yielding is the supported pattern now.
```suggestion
},
// Note for cpp-extension authors: MinifiProcessorCallbacks no longer
exposes an
// isWorkAvailable callback, so overriding
ProcessorImpl::isWorkAvailable() has no
// effect on scheduling. Use getTriggerWhenEmpty() together with
yielding from
// onTrigger()/onSchedule() as the supported pattern for controlling
rescheduling.
```
##########
libminifi/include/utils/CProcessor.h:
##########
@@ -81,7 +81,7 @@ class CProcessor : public minifi::core::ProcessorApi {
}
Review Comment:
`CProcessor::isWorkAvailable()` is now hard-coded to return `false`, which
removes the ability for C/C++ extensions (via the C API) to signal
internally-generated work (i.e., work not represented by incoming connections).
If this removal is intentional, it should be made explicit at the API boundary
so extension authors don’t assume there is still a way to implement efficient
event-driven behavior.
Suggested fix: add a short comment here (and/or in `minifi-c.h`) stating
that `isWorkAvailable` is not supported in API v3 and that extensions should
use `getTriggerWhenEmpty` combined with yielding from `trigger()` when idle;
alternatively, if internal work signaling is still needed, keep an explicit
callback for it.
```suggestion
// API v3 does not support internal work signaling for C processors via
isWorkAvailable().
// This intentionally always returns false. Extensions that need to run
without incoming
// connections should use getTriggerWhenEmpty and yield from trigger()
when idle.
```
--
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]
