-
Notifications
You must be signed in to change notification settings - Fork 25.2k
[ML] adding running_state to datafeed stats object #73926
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ML] adding running_state to datafeed stats object #73926
Conversation
Pinging @elastic/ml-core (Team:ML) |
adcaf29
to
ff479e7
Compare
has no configured `end` time. | ||
|
||
`finished_look_back`::: | ||
(boolean) Has the {dfeed} finished running on the available past data. For {dfeeds} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(boolean) Has the {dfeed} finished running on the available past data. For {dfeeds} | |
(boolean) Indicates whether the {dfeed} has finished running on the available past data. For {dfeeds} |
|
||
`finished_look_back`::: | ||
(boolean) Has the {dfeed} finished running on the available past data. For {dfeeds} | ||
that without a configured `end` time, this means that the {dfeed} is now running on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that without a configured `end` time, this means that the {dfeed} is now running on | |
without a configured `end` time, this means that the {dfeed} is now running on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
My only issue is whether we introduce the term "lookback" to the world. I am leaning towards not doing this.
(boolean) Indicates if the {dfeed} is "real-time"; meaning that the {dfeed} | ||
has no configured `end` time. | ||
|
||
`finished_look_back`::: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our existing docs don't use the term "lookback". I am not sure we should introduce it now.
One solution would be to rename is_real_time
to real_time_configured
and finished_look_back
to real_time_running
. Then that doesn't involve introducing a new public term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While its true that we don't use it in docs, lookback
is used in audit messages:
public static final String JOB_AUDIT_DATAFEED_LOOKBACK_COMPLETED = "Datafeed lookback completed";
public static final String JOB_AUDIT_DATAFEED_LOOKBACK_NO_DATA = "Datafeed lookback retrieved no data";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am happy to change to both to indicate real_time
. But we do use lookback in user facing things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lookback
is used in audit messages
Oh, interesting. I think we made the decision in 2016 not to use it anywhere user-facing, but those audit messages were added in 2017, by which time the 2016 decision had been forgotten. I guess the lesson is that we should change our terminology in internal code as well as what's immediately user-facing to stop internal terminology leaking out later on.
I still think there is a benefit in not propagating the term to fields that will appear in every high level client's public API. At the moment we could change the wording of those audit messages without making a breaking change, but the REST responses are harder to change once published.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okey dokey, I will rename the variables.
I guess the lesson is that we should change our terminology in internal code as well as what's immediately user-facing to stop internal terminology leaking out later on.
100%. Renaming things everywhere prevents this sort of thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@elasticmachine update branch |
Pinging @elastic/clients-team (Team:Clients) |
It is useful to know the following information when reading datafeed stats: - Is the datafeed a "real-time" datafeed, i.e. a datafeed without a configured `end` time - Has the datafeed processed all past data available at the time of starting. This object is only available if the datafeed task has been created. It has the form: ``` "running_state": { "is_real_time": <boolean>, "look_back_finished": <boolean> } ```
It is useful to know the following information when reading datafeed stats:
end
timeThis object is only available if the datafeed task has been created.
It has the form: