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.

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][Data Frame] Add version and create_time to transform config #43384

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,20 @@

package org.elasticsearch.client.dataframe.transforms;

import org.elasticsearch.Version;
import org.elasticsearch.client.dataframe.transforms.pivot.PivotConfig;
import org.elasticsearch.client.dataframe.transforms.util.TimeUtil;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;

import java.io.IOException;
import java.util.Date;
import java.util.Objects;

import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
Expand All @@ -40,6 +44,8 @@ public class DataFrameTransformConfig implements ToXContentObject {
public static final ParseField SOURCE = new ParseField("source");
public static final ParseField DEST = new ParseField("dest");
public static final ParseField DESCRIPTION = new ParseField("description");
public static final ParseField TRANSFORM_VERSION = new ParseField("transform_version");
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should call this just version. We went for job_version for anomaly detection jobs in hindsight it seems redundant.

Copy link
Member Author

Choose a reason for hiding this comment

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

Eh, I am fine with changing it.

public static final ParseField CREATE_TIME = new ParseField("create_time");
// types of transforms
public static final ParseField PIVOT_TRANSFORM = new ParseField("pivot");

Expand All @@ -48,6 +54,8 @@ public class DataFrameTransformConfig implements ToXContentObject {
private final DestConfig dest;
private final PivotConfig pivotConfig;
private final String description;
private final Version transformVersion;
private final Date createTime;
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is new code, should we use the new java time classes (I guess Instant in this case)?

Copy link
Member Author

@benwtrent benwtrent Jun 20, 2019

Choose a reason for hiding this comment

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

@dimitris-athanasiou is this a thing? I did not know we should be using Instant instead of Date and that one supplanted the other. TIL.

Copy link
Contributor

@droberts195 droberts195 Jun 20, 2019

Choose a reason for hiding this comment

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

Yes, use Instant and serialize it using StreamOutput.writeInstant() and StreamInput.readInstant() (or StreamOutput.writeOptionalInstant() and StreamInput.readOptionalInstant() if more appropriate).


public static final ConstructingObjectParser<DataFrameTransformConfig, Void> PARSER =
new ConstructingObjectParser<>("data_frame_transform", true,
Expand All @@ -57,7 +65,9 @@ public class DataFrameTransformConfig implements ToXContentObject {
DestConfig dest = (DestConfig) args[2];
PivotConfig pivotConfig = (PivotConfig) args[3];
String description = (String)args[4];
return new DataFrameTransformConfig(id, source, dest, pivotConfig, description);
Date createTime = (Date)args[5];
String transformVersion = (String)args[6];
return new DataFrameTransformConfig(id, source, dest, pivotConfig, description, createTime, transformVersion);
});

static {
Expand All @@ -66,6 +76,9 @@ public class DataFrameTransformConfig implements ToXContentObject {
PARSER.declareObject(constructorArg(), (p, c) -> DestConfig.PARSER.apply(p, null), DEST);
PARSER.declareObject(optionalConstructorArg(), (p, c) -> PivotConfig.fromXContent(p), PIVOT_TRANSFORM);
PARSER.declareString(optionalConstructorArg(), DESCRIPTION);
PARSER.declareField(optionalConstructorArg(),
p -> TimeUtil.parseTimeField(p, CREATE_TIME.getPreferredName()), CREATE_TIME, ObjectParser.ValueType.VALUE);
PARSER.declareString(optionalConstructorArg(), TRANSFORM_VERSION);
}

public static DataFrameTransformConfig fromXContent(final XContentParser parser) {
Expand All @@ -84,19 +97,23 @@ public static DataFrameTransformConfig fromXContent(final XContentParser parser)
* @return A DataFrameTransformConfig to preview, NOTE it will have a {@code null} id, destination and index.
*/
public static DataFrameTransformConfig forPreview(final SourceConfig source, final PivotConfig pivotConfig) {
return new DataFrameTransformConfig(null, source, null, pivotConfig, null);
return new DataFrameTransformConfig(null, source, null, pivotConfig, null, null, null);
}

DataFrameTransformConfig(final String id,
final SourceConfig source,
final DestConfig dest,
final PivotConfig pivotConfig,
final String description) {
final String description,
final Date createTime,
final String version) {
this.id = id;
this.source = source;
this.dest = dest;
this.pivotConfig = pivotConfig;
this.description = description;
this.createTime = createTime;
this.transformVersion = version == null ? null : Version.fromString(version);
}

public String getId() {
Expand All @@ -115,6 +132,14 @@ public PivotConfig getPivotConfig() {
return pivotConfig;
}

public Version getTransformVersion() {
return transformVersion;
}

public Date getCreateTime() {
return createTime;
}

@Nullable
public String getDescription() {
return description;
Expand All @@ -138,6 +163,12 @@ public XContentBuilder toXContent(final XContentBuilder builder, final Params pa
if (description != null) {
builder.field(DESCRIPTION.getPreferredName(), description);
}
if (createTime != null) {
builder.timeField(CREATE_TIME.getPreferredName(), CREATE_TIME.getPreferredName() + "_string", createTime.getTime());
}
if (transformVersion != null) {
builder.field(TRANSFORM_VERSION.getPreferredName(), transformVersion);
}
builder.endObject();
return builder;
}
Expand All @@ -155,15 +186,17 @@ public boolean equals(Object other) {
final DataFrameTransformConfig that = (DataFrameTransformConfig) other;

return Objects.equals(this.id, that.id)
&& Objects.equals(this.source, that.source)
&& Objects.equals(this.dest, that.dest)
&& Objects.equals(this.description, that.description)
&& Objects.equals(this.pivotConfig, that.pivotConfig);
&& Objects.equals(this.source, that.source)
&& Objects.equals(this.dest, that.dest)
&& Objects.equals(this.description, that.description)
&& Objects.equals(this.transformVersion, that.transformVersion)
&& Objects.equals(this.createTime, that.createTime)
&& Objects.equals(this.pivotConfig, that.pivotConfig);
}

@Override
public int hashCode() {
return Objects.hash(id, source, dest, pivotConfig, description);
return Objects.hash(id, source, dest, pivotConfig, description, createTime, transformVersion);
}

@Override
Expand Down Expand Up @@ -209,7 +242,7 @@ public Builder setDescription(String description) {
}

public DataFrameTransformConfig build() {
return new DataFrameTransformConfig(id, source, dest, pivotConfig, description);
return new DataFrameTransformConfig(id, source, dest, pivotConfig, description, null, null);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.client.dataframe.transforms.util;

import org.elasticsearch.common.time.DateFormatters;
import org.elasticsearch.common.xcontent.XContentParser;

import java.io.IOException;
import java.time.format.DateTimeFormatter;
import java.util.Date;

public final class TimeUtil {

/**
* Parse out a Date object given the current parser and field name.
*
* @param parser current XContentParser
* @param fieldName the field's preferred name (utilized in exception)
* @return parsed Date object
* @throws IOException from XContentParser
*/
public static Date parseTimeField(XContentParser parser, String fieldName) throws IOException {
if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) {
return new Date(parser.longValue());
} else if (parser.currentToken() == XContentParser.Token.VALUE_STRING) {
return new Date(DateFormatters.from(DateTimeFormatter.ISO_INSTANT.parse(parser.text())).toInstant().toEpochMilli());
}
throw new IllegalArgumentException(
"unexpected token [" + parser.currentToken() + "] for [" + fieldName + "]");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public void testGetTransform() throws IOException {
client::getDataFrameTransformAsync);
assertNull(getResponse.getInvalidTransforms());
assertThat(getResponse.getTransformConfigurations(), hasSize(1));
assertEquals(transform, getResponse.getTransformConfigurations().get(0));
assertEquals(transform.getId(), getResponse.getTransformConfigurations().get(0).getId());
}

public void testGetAllAndPageTransforms() throws IOException {
Expand All @@ -219,7 +219,7 @@ public void testGetAllAndPageTransforms() throws IOException {
client::getDataFrameTransformAsync);
assertNull(getResponse.getInvalidTransforms());
assertThat(getResponse.getTransformConfigurations(), hasSize(2));
assertEquals(transform, getResponse.getTransformConfigurations().get(1));
assertEquals(transform.getId(), getResponse.getTransformConfigurations().get(1).getId());

getRequest.setPageParams(new PageParams(0,1));
getResponse = execute(getRequest, client::getDataFrameTransform,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.client.dataframe.transforms;

import org.elasticsearch.Version;
import org.elasticsearch.client.dataframe.transforms.pivot.PivotConfigTests;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
Expand All @@ -28,6 +29,7 @@

import java.io.IOException;
import java.util.Collections;
import java.util.Date;
import java.util.function.Predicate;

import static org.elasticsearch.client.dataframe.transforms.DestConfigTests.randomDestConfig;
Expand All @@ -36,8 +38,13 @@
public class DataFrameTransformConfigTests extends AbstractXContentTestCase<DataFrameTransformConfig> {

public static DataFrameTransformConfig randomDataFrameTransformConfig() {
return new DataFrameTransformConfig(randomAlphaOfLengthBetween(1, 10), randomSourceConfig(),
randomDestConfig(), PivotConfigTests.randomPivotConfig(), randomBoolean() ? null : randomAlphaOfLengthBetween(1, 100));
return new DataFrameTransformConfig(randomAlphaOfLengthBetween(1, 10),
randomSourceConfig(),
randomDestConfig(),
PivotConfigTests.randomPivotConfig(),
randomBoolean() ? null : randomAlphaOfLengthBetween(1, 100),
randomBoolean() ? null : new Date(),
randomBoolean() ? null : Version.CURRENT.toString());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,6 @@ public void testGetStats() throws IOException, InterruptedException {

RestHighLevelClient client = highLevelClient();

QueryConfig queryConfig = new QueryConfig(new MatchAllQueryBuilder());
GroupConfig groupConfig = GroupConfig.builder().groupBy("reviewer",
TermsGroupSource.builder().setField("user_id").build()).build();
AggregatorFactories.Builder aggBuilder = new AggregatorFactories.Builder();
Expand Down Expand Up @@ -559,7 +558,6 @@ public void onFailure(Exception e) {
public void testGetDataFrameTransform() throws IOException, InterruptedException {
createIndex("source-data");

QueryConfig queryConfig = new QueryConfig(new MatchAllQueryBuilder());
GroupConfig groupConfig = GroupConfig.builder().groupBy("reviewer",
TermsGroupSource.builder().setField("user_id").build()).build();
AggregatorFactories.Builder aggBuilder = new AggregatorFactories.Builder();
Expand Down
Loading