-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Associate an external id with a task, so that it can be cancelled later. #3277
Conversation
public bool Cancellable { get; internal set; } | ||
|
||
[JsonProperty("headers")] | ||
public IReadOnlyDictionary<string, string> Headers { get; internal set; } = EmptyReadOnly<string, string>.Dictionary; |
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, do we know what headers are possibly returned here?
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.
Sadly not (from the source code I read) but certainly X-Opaque-Id
.
I had considered a helper method to expose this value, but just left to the user for the time being...
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.
Generally LGTM, left some comments. It looks like TasKInfo
is missing child tasks
/// Associate an Id with this user-initiated task, such that it can be located in the cluster task list. | ||
/// <pre>https://www.elastic.co/guide/en/elasticsearch/reference/master/tasks.html#_identifying_running_tasks</pre> | ||
/// </summary> | ||
string OpaqueId { get; set; } |
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.
Add something to indicate that this is valid only for Elasticsearch 6.2.0+
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.
++
/// Associate an Id with this user-initiated task, such that it can be located in the cluster task list. | ||
/// <pre>https://www.elastic.co/guide/en/elasticsearch/reference/6.2/tasks.html#_identifying_running_tasks</pre> | ||
/// </summary> | ||
public RequestConfigurationDescriptor OpaqueId(string opaqueId) |
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.
Add something to indicate that this is valid only for Elasticsearch 6.2.0+
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.
++
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 think rather than having a link to docs, something like "Valid for Elasticsearch 6.2.0+" would be sufficient.
@@ -36,5 +36,8 @@ public class TaskInfo | |||
|
|||
[JsonProperty("cancellable")] | |||
public bool Cancellable { get; internal set; } | |||
|
|||
[JsonProperty("headers")] | |||
public IReadOnlyDictionary<string, string> Headers { get; internal set; } = EmptyReadOnly<string, string>.Dictionary; | |||
} |
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.
missing child tasks in children
array?
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.
Good catch, have added.
Backported to |
Closes #3259