Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ML][Data Frame] Add support for allow_no_match for endpoints #43490

Merged
merged 6 commits into from
Jun 26, 2019
Merged

[ML][Data Frame] Add support for allow_no_match for endpoints #43490

merged 6 commits into from
Jun 26, 2019

Conversation

benwtrent
Copy link
Member

@benwtrent benwtrent commented Jun 21, 2019 •
edited
Loading

I opted for the name allow_no_transforms in the APIs. This is for parity with ml jobs and datafeeds.

The only peculiar endpoint is _stats. If nothing is found, the current behavior is NOT to throw, but to return empty. Do we want to support this parameter in that API and do we want it to cause that endpoint to return a 404? I basically added the parameter, but it does not do much really.

closes #42766

@benwtrent
[ML][Data Frame] Add support for allow_no_transforms for endpoints
9c49767 
Adds support for:
* Get Transforms
* Get Transforms stats
* stop transforms
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

@dimitris-athanasiou
Copy link
Contributor

I suggest we drop the paradigm we followed for jobs and datafeeds and adopt allow_no_match from now on. The paradigm of allow_no_<resource_name> is introducing a new parameter name each time we add a resource and it doesn't work great with long named resources. I also think allow_no_match better describes what the parameter actually does. We can reestablish consistency by deprecating the current parameters for jobs and datafeeds and introduce allow_no_match there as well.

@benwtrent
Copy link
Member Author

@dimitris-athanasiou I am all for it. I will adjust this PR :)

@droberts195 droberts195 changed the title [ML][Data Frame] Add support for allow_no_transforms for endpoints [ML][Data Frame] Add support for allow_no_match for endpoints Jun 25, 2019
droberts195
droberts195 reviewed Jun 25, 2019
View reviewed changes
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few places, admittedly not user facing, where the old names are still used. But for the sake of maintainers I think it would be nice to make all the names consistent.

...igh-level/src/main/java/org/elasticsearch/client/dataframe/GetDataFrameTransformRequest.java Outdated
@@ -40,6 +41,7 @@ public static GetDataFrameTransformRequest getAllDataFrameTransformsRequest() {

private final List<String> ids;
private PageParams pageParams;
private Boolean allowNoTransforms;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be less confusing to also rename the member variable.

...evel/src/main/java/org/elasticsearch/client/dataframe/GetDataFrameTransformStatsRequest.java Outdated
@@ -29,6 +29,7 @@
public class GetDataFrameTransformStatsRequest implements Validatable {
private final String id;
private PageParams pageParams;
private Boolean allowNoTransforms;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be less confusing to also rename the member variable.

...gh-level/src/main/java/org/elasticsearch/client/dataframe/StopDataFrameTransformRequest.java Outdated
@@ -31,6 +31,7 @@
private final String id;
private Boolean waitForCompletion;
private TimeValue timeout;
private Boolean allowNoTransforms;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be less confusing to also rename the member variable.

...in/java/org/elasticsearch/xpack/core/dataframe/action/GetDataFrameTransformsStatsAction.java Outdated
@@ -55,6 +55,7 @@ public Response newResponse() {
public static class Request extends BaseTasksRequest<Request> {
private final String id;
private PageParams pageParams = PageParams.defaultParams();
private boolean allowNoResources = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename allowNoResources to allowNoMatch throughout this file, including the getter and setter.

...rc/main/java/org/elasticsearch/xpack/core/dataframe/action/StopDataFrameTransformAction.java Outdated
@@ -56,15 +57,17 @@ public Response newResponse() {
private final String id;
private final boolean waitForCompletion;
private final boolean force;
private final boolean allowNoResources;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename allowNoResources to allowNoMatch throughout this file, including the getter and setter.

@benwtrent
Copy link
Member Author

@elasticmachine update branch

droberts195
droberts195 approved these changes Jun 26, 2019
View reviewed changes
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@benwtrent benwtrent merged commit 9084418 into elastic:master Jun 26, 2019
@benwtrent benwtrent deleted the feature/ml-df-add-allow_no_match branch June 26, 2019 11:55
benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Jun 26, 2019
@benwtrent
[ML][Data Frame] Add support for allow_no_match for endpoints (elasti…
3a9ffc8 
…c#43490)

* [ML][Data Frame] Add support for allow_no_match parameter in endpoints

Adds support for:
* Get Transforms
* Get Transforms stats
* stop transforms
@benwtrent benwtrent mentioned this pull request Jun 26, 2019
Merged
benwtrent added a commit that referenced this pull request Jun 26, 2019
@benwtrent
[7.x] [ML][Data Frame] Add support for allow_no_match for endpoints (#…
c121b00 
…43490) (#43637)

* [ML][Data Frame] Add support for allow_no_match for endpoints (#43490)

* [ML][Data Frame] Add support for allow_no_match parameter in endpoints

Adds support for:
* Get Transforms
* Get Transforms stats
* stop transforms

* Update DataFrameTransformDocumentationIT.java
@lcawl lcawl mentioned this pull request Jun 26, 2019
Merged
@Mpdreamz Mpdreamz mentioned this pull request Aug 7, 2019
Closed
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@droberts195 droberts195 droberts195 approved these changes

Assignees
No one assigned
Labels
>enhancement :ml/Transform Transform v7.3.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ML] Add allow_no_match param to data frame transform APIs
5 participants
@benwtrent @elasticmachine @dimitris-athanasiou @droberts195 @jakelandis