[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user asfgit closed the pull request at: https://github.com/apache/metron/pull/662 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user james-sirota commented on a diff in the pull request: https://github.com/apache/metron/pull/662#discussion_r130675495 --- Diff: metron-platform/metron-indexing/src/test/java/org/apache/metron/indexing/dao/IndexingDaoIntegrationTest.java --- @@ -27,28 +28,32 @@ import org.json.simple.parser.ParseException; import org.junit.*; +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.Map; --- End diff -- @ottobackwards are you ok with this PR? Can this get a +1 from you? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron/pull/662#discussion_r129156078
--- Diff:
metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/search/FieldType.java
---
@@ -0,0 +1,52 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF 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.apache.metron.indexing.dao.search;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+public enum FieldType {
+ @JsonProperty("string")
+ STRING("string"),
+ @JsonProperty("ip")
+ IP("ip"),
+ @JsonProperty("integer")
+ INTEGER("integer"),
+ @JsonProperty("long")
+ LONG("long"),
+ @JsonProperty("date")
+ DATE("date"),
+ @JsonProperty("float")
+ FLOAT("float"),
+ @JsonProperty("double")
+ DOUBLE("double"),
+ @JsonProperty("boolean")
+ BOOLEAN("boolean"),
+ @JsonProperty("other")
--- End diff --
Just to be clear, I don't think this should hold up this PR, but that we in
a general sense need to go into more detail.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user ottobackwards commented on a diff in the pull request: https://github.com/apache/metron/pull/662#discussion_r129152184 --- Diff: metron-platform/metron-indexing/src/test/java/org/apache/metron/indexing/dao/IndexingDaoIntegrationTest.java --- @@ -27,28 +28,32 @@ import org.json.simple.parser.ParseException; import org.junit.*; +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.Map; --- End diff -- Yes --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/662#discussion_r129147257
--- Diff:
metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/search/FieldType.java
---
@@ -0,0 +1,52 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF 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.apache.metron.indexing.dao.search;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+public enum FieldType {
+ @JsonProperty("string")
+ STRING("string"),
+ @JsonProperty("ip")
+ IP("ip"),
+ @JsonProperty("integer")
+ INTEGER("integer"),
+ @JsonProperty("long")
+ LONG("long"),
+ @JsonProperty("date")
+ DATE("date"),
+ @JsonProperty("float")
+ FLOAT("float"),
+ @JsonProperty("double")
+ DOUBLE("double"),
+ @JsonProperty("boolean")
+ BOOLEAN("boolean"),
+ @JsonProperty("other")
--- End diff --
"other" just means it's not one of the other simple types in the enum. The
Elasticsearch Java API returns the search result as a Map type
which is also the return type of the controller so it's up to Jackson to
serialize it. We are not doing anything special to map values to types when
returning search results. So far I haven't seen Jackson have any problems with
the types being returned. From what I can tell Elasticsearch mostly sticks to
simple types for the values.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/662#discussion_r129145507
--- Diff:
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/SearchController.java
---
@@ -45,4 +49,18 @@
ResponseEntity search(final @ApiParam(name =
"searchRequest", value = "Search request", required = true) @RequestBody
SearchRequest searchRequest) throws RestException {
return new ResponseEntity<>(searchService.search(searchRequest),
HttpStatus.OK);
}
+
--- End diff --
It's a legitimate concern but I think synchronous is ok here because
Elasticsearch is intended to provide results in real-time. The http javascript
APIs we use are asynchronous so it shouldn't make a difference in the UI. If
this were a batch operation (Hive, Map Reduce) I would agree that it should be
an async call.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user merrimanr commented on a diff in the pull request: https://github.com/apache/metron/pull/662#discussion_r129143530 --- Diff: metron-interface/metron-rest/README.md --- @@ -353,6 +355,20 @@ Request and Response objects are JSON formatted. The JSON schemas are available * searchRequest - Search request * Returns: * 200 - Search results + --- End diff -- This isn't intended to be the source of truth for types that we support. I would think any data type someone may need should be supported in some way. This is restricted to (mostly) simple types and is driven by the need to format simple values in different ways. Anything else besides simple types is categorized as "other". If at any point there is a need to include more types this can easily be extended. The Elasticsearch API gives you either a number or string (only string in the case of the REST API) so we need more information about the type in Elasticsearch to format it correctly. Solr is similar. I will take another pass at the documentation to make this more obvious. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user ottobackwards commented on a diff in the pull request: https://github.com/apache/metron/pull/662#discussion_r129051676 --- Diff: metron-interface/metron-rest/README.md --- @@ -353,6 +355,20 @@ Request and Response objects are JSON formatted. The JSON schemas are available * searchRequest - Search request * Returns: * 200 - Search results + --- End diff -- This is not a problem for this PR, but rather the documentation in general. The actual TYPES returned from the rest calls are not documented at all. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron/pull/662#discussion_r129051990
--- Diff:
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/SearchController.java
---
@@ -45,4 +49,18 @@
ResponseEntity search(final @ApiParam(name =
"searchRequest", value = "Search request", required = true) @RequestBody
SearchRequest searchRequest) throws RestException {
return new ResponseEntity<>(searchService.search(searchRequest),
HttpStatus.OK);
}
+
--- End diff --
Is this operation very fast? or should it be a candidate for an async call?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user ottobackwards commented on a diff in the pull request: https://github.com/apache/metron/pull/662#discussion_r129052380 --- Diff: metron-interface/metron-rest/README.md --- @@ -353,6 +355,20 @@ Request and Response objects are JSON formatted. The JSON schemas are available * searchRequest - Search request * Returns: * 200 - Search results + --- End diff -- Such as complex types. This comes through here, because we are creating our own Field Type Enums --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user ottobackwards commented on a diff in the pull request: https://github.com/apache/metron/pull/662#discussion_r129055024 --- Diff: metron-platform/metron-indexing/src/test/java/org/apache/metron/indexing/dao/IndexingDaoIntegrationTest.java --- @@ -27,28 +28,32 @@ import org.json.simple.parser.ParseException; import org.junit.*; +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.Map; --- End diff -- How do we test unknown --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron/pull/662#discussion_r129054322
--- Diff:
metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/search/FieldType.java
---
@@ -0,0 +1,52 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF 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.apache.metron.indexing.dao.search;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+public enum FieldType {
+ @JsonProperty("string")
+ STRING("string"),
+ @JsonProperty("ip")
+ IP("ip"),
+ @JsonProperty("integer")
+ INTEGER("integer"),
+ @JsonProperty("long")
+ LONG("long"),
+ @JsonProperty("date")
+ DATE("date"),
+ @JsonProperty("float")
+ FLOAT("float"),
+ @JsonProperty("double")
+ DOUBLE("double"),
+ @JsonProperty("boolean")
+ BOOLEAN("boolean"),
+ @JsonProperty("other")
--- End diff --
What does other mean? Is it mapped to string in return values..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] metron pull request #662: METRON-1056: Get field types from Elasticsearch
Github user ottobackwards commented on a diff in the pull request: https://github.com/apache/metron/pull/662#discussion_r129052922 --- Diff: metron-interface/metron-rest/README.md --- @@ -353,6 +355,20 @@ Request and Response objects are JSON formatted. The JSON schemas are available * searchRequest - Search request * Returns: * 200 - Search results + --- End diff -- We may want to consider explaining the types we support and return here... but I am not sure it should be inline. Since the detail is in line with the rest of the document, maybe that can be a follow on --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
