davsclaus commented on code in PR #27908:
URL: https://github.com/apache/camel/pull/27908#discussion_r4286226202
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc:
##########
@@ -866,10 +866,14 @@ with `--include-input` for case/bulk requests that need
captured input. Existing
=== camel-milo - creating and deleting the monitored items of a consumer
Since 4.16, a `milo-client` consumer whose monitored item could not be
created, for example because the node does not
-exist, received nothing. Now it receives one `DataValue` with the bad status
code (when `omitNullValues=false`). The
+exist, received nothing. Now it receives one `DataValue` with the bad status
code. The
monitored item is created again when the route is restarted or when Camel
rebuilds the connection to the server; unlike
Camel 3.x, it is not retried every 10 seconds.
+A data change with a Bad or Uncertain status is now delivered by default, even
when its value is null; previously the
+default `omitNullValues=true` dropped it, so the route did not learn that a
node went bad. `omitNullValues` now only
+omits a data change whose value is null and whose status is Good, for example
the initial value at startup.
Review Comment:
Users upgrading most need to know that a route may now receive a Bad status
at startup, for example from a `milo-server` item that was not written yet, or
a server that reports `Bad_WaitingForInitialData`. Previously the default
dropped it. Could you say that here, and how to skip it (check
`getStatusCode()` of the `DataValue` body)? The sentence "for example the
initial value at startup" currently suggests the opposite.
##########
components/camel-milo/src/main/java/org/apache/camel/component/milo/client/MiloClientEndpoint.java:
##########
@@ -91,7 +91,8 @@ public class MiloClientEndpoint extends DefaultEndpoint {
private boolean defaultAwaitWrites;
/**
- * Omit notifications in case of null values
+ * Omit a data change whose value is null and whose status is Good, for
example the initial value at startup. A data
Review Comment:
"for example the initial value at startup": for a `milo-server` item that
has not been written yet, the startup value has a Bad status (`CamelServerItem`
starts with `new DataValue(StatusCode.BAD)`), so it is delivered, not omitted.
Maybe drop the example, or say that a Bad startup value such as
`Bad_WaitingForInitialData` is delivered. This text feeds the generated JSON,
the catalog and the endpoint DSL.
##########
components/camel-milo/src/main/java/org/apache/camel/component/milo/client/MiloClientConsumer.java:
##########
@@ -131,6 +131,15 @@ private void handleValueUpdate(final String node, final
DataValue value) {
}
}
+ /**
+ * A null value with a Good status is the initial value at startup, which
omitNullValues hides. A null value with a
Review Comment:
The startup value is not always a Good null. `CamelServerItem` starts every
item as `new DataValue(StatusCode.BAD)`, and a server may report
`Bad_WaitingForInitialData`. In both cases this predicate returns `false`, so
the startup notification is now delivered. That is fine if it is the intent,
but this javadoc (and the option description) should not say the startup value
is what gets omitted. See the review summary for the existing tests that seem
to depend on it.
--
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]
