You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by GitBox <gi...@apache.org> on 2021/02/20 18:31:24 UTC
[GitHub] [zeppelin] smiklosovic opened a new pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
smiklosovic opened a new pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057
### What is this PR for?
This PR fixes issus for search command in Elasticsearch as it is not working now. This is rather a serious problem as right now, ES interpreter is more or less useless when one can not search in it so it would be awesome if it is merged as not doing so renders that plugin useless.
The problem is that in the past, there was this in the response from a server on search:
```
{
"took": 1,
"timed_out": false,
"_shards": {
"total": 1,
"successful": 1,
"skipped": 0,
"failed": 0
},
"hits": {
"total": 1
},
/// other fields omitted for brevity
```
but now it returns:
```
{
"took": 1,
"timed_out": false,
"_shards": {
"total": 1,
"successful": 1,
"skipped": 0,
"failed": 0
},
"hits": {
"total": {
"value": 1,
"relation": "eq"
},
/// other fields omitted for brevity
```
This changed behaviour seems to be introduced in recent ES versions 7.x, I do not have any capability to check how far into the past this issue is present, I have done it in such a way that it is backward-compatible with older versions too.
### What type of PR is it?
Bug Fix.
### What is the Jira issue?
ZEPPELIN-4843
### How should this be tested?
* I have run unit tests in Java for ES interpreter, they all worked.
### Questions:
* Does the licenses files need update? - no
* Is there breaking changes for older versions? - no, the older version is still supported, the solution works as it was but it fallbacks to new way of doing things for more recent ES versions.
* Does this needs documentation? - no
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] smiklosovic commented on pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
smiklosovic commented on pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057#issuecomment-783315201
@zjffdu I have added javadoc and squashed the changes into 1 commit. Please review and eventually merge.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057#issuecomment-782766253
Thanks @smiklosovic for the contribution, PR LGTM, only a minor comment
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057#issuecomment-783202465
Maybe it's time to update our Elasticsearch library.
We are currently using [2.4.3](https://github.com/apache/zeppelin/blob/master/elasticsearch/pom.xml#L37), which was release in December 2016.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] asfgit closed pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] smiklosovic commented on pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
smiklosovic commented on pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057#issuecomment-784955399
@zjffdu is there anything else I should do in this matter?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057#issuecomment-783450290
@smiklosovic CI is failed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on a change in pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
zjffdu commented on a change in pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057#discussion_r579724173
##########
File path: elasticsearch/src/main/java/org/apache/zeppelin/elasticsearch/client/HttpBasedClient.java
##########
@@ -254,6 +255,15 @@ public ActionResponse index(String index, String type, String id, String data) {
return response;
}
+ private long getTotal(HttpResponse<JsonNode> result) {
+ final JSONObject hitsObject = result.getBody().getObject().getJSONObject("hits");
+ try {
+ return hitsObject.getLong("total");
+ } catch (final JSONException ex) {
+ return hitsObject.getJSONObject("total").getLong("value");
Review comment:
Could you add one comment to explain the backward compatibility logic here ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] smiklosovic commented on pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
smiklosovic commented on pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057#issuecomment-783299690
@Reamer btw, the transport client is becoming obsolete these days, if you check e.g. spring data for elasticsearch they are already marking it as obsolete, maybe a ticket which just deletes it wont hurt, I tried transport client in Zeppelin for 7.11.0 and it was throwing exceptions etc ...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057#issuecomment-786540179
LGTM， will merge if no more comment
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] smiklosovic commented on pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
smiklosovic commented on pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057#issuecomment-783599619
hi @zjffdu , I have fixed my errors, it was about Javadoc being longer tha 120 chars (something like that). I do not know how to influence other modules. It seems that Zeppelin has just a flaky tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] smiklosovic commented on pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
smiklosovic commented on pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057#issuecomment-786225469
@zjffdu is there anything else I should do in this matter?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] smiklosovic edited a comment on pull request #4057: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
Posted by GitBox <gi...@apache.org>.
smiklosovic edited a comment on pull request #4057:
URL: https://github.com/apache/zeppelin/pull/4057#issuecomment-783599619
hi @zjffdu , I have fixed my errors, it was about Javadoc being longer than 120 chars (something like that). I do not know how to influence other modules. It seems that Zeppelin has just a flaky tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org