Integration of ElasticSearch v1.0 (GROUND - 6)#68
Integration of ElasticSearch v1.0 (GROUND - 6)#68nipunramk wants to merge 36 commits intoground-context:masterfrom nipunramk:master
Conversation
vsreekanti
left a comment
vsreekanti
left a comment
There was a problem hiding this comment.
Bunch of comments inline. Tests are also failing on the CI build.
pom.xml
Outdated
| <dependency> | ||
| <groupId>org.elasticsearch</groupId> | ||
| <artifactId>elasticsearch</artifactId> | ||
| <version>1.5.1</version> |
There was a problem hiding this comment.
The version should be a variable in the properties field of the POM.
| import edu.berkeley.ground.dao.models.NodeVersionFactory; | ||
| import edu.berkeley.ground.dao.models.StructureFactory; | ||
| import edu.berkeley.ground.dao.models.StructureVersionFactory; | ||
| import edu.berkeley.ground.dao.models.*; |
There was a problem hiding this comment.
We're using the Google Java style guide which specifies no * imports.
| public abstract List<Long> getVersionIdsByTag(String tag) throws GroundException; | ||
|
|
||
| public abstract List<Long> getItemIdsByTag(String tag) throws GroundException; | ||
|
|
There was a problem hiding this comment.
Please remove unnecessary whitespace.
| @Override | ||
| public List<Long> getVersionIdsByTag(String tag) throws GroundException { | ||
| return this.getIdsByTag(tag, "rich_version"); | ||
| return ElasticSearch.getSearchResponse("rich_version", tag); |
There was a problem hiding this comment.
We should probably have a configuration option that allows people to turn on / off ElasticSearch use. Let's add a field to the config and pass it through to the TagFactory. If it's turned off, we can go to the regular database (current impl.), and if it's turned on, we can use the ElasticSearch API.
| LOGGER.info("Retrieving all item ids with tag: " + tag + "."); | ||
| return this.tagFactory.getItemIdsByTag(tag); | ||
| } | ||
|
|
| String json = mapper.writeValueAsString(tag); | ||
| IndexResponse response = client.prepareIndex(clusterName, table, Long.toString(tag.getId())) | ||
| .setSource(json).get(); | ||
| client.admin().indices().prepareRefresh().execute().actionGet(); // need to refresh index with new inserted item |
There was a problem hiding this comment.
What exactly does refreshing an index do? Do we need to do it on every insert?
| return response.isCreated(); | ||
|
|
||
| } catch (JsonProcessingException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
Printing the stack trace isn't the right thing to do. We should pass the error information through with the exception that we throw.
|
|
||
| public static List<Long> getSearchResponse(String type, String searchQuery) throws GroundException { | ||
| SearchResponse response = client.prepareSearch().setTypes(type).setQuery(QueryBuilders.matchQuery("key", searchQuery)).get(); | ||
| SearchHit[] hits = response.getHits().getHits(); |
There was a problem hiding this comment.
Is .getHits().getHits() an ES API idiosyncrasy?
| import edu.berkeley.ground.dao.models.NodeVersionFactory; | ||
| import edu.berkeley.ground.dao.models.StructureFactory; | ||
| import edu.berkeley.ground.dao.models.StructureVersionFactory; | ||
| import edu.berkeley.ground.dao.models.*; |
|
|
||
| @BeforeClass | ||
| public static void setup() throws GroundDbException { | ||
| public static void setup() throws GroundDbException, GroundException { |
There was a problem hiding this comment.
Shouldn't throw both. GroundDbException is a subclass of GroundException.
|
Please also put the name of the JIRA issue (GROUND-6) in square brackets at the beginning of the PR for consistency. :-) |
|
@nipunramk: Looks like the build is still failing. |
nipunramk
changed the title
2 participants
Added ability to insert into ElasticSearch and get search responses by key values in tags and also by ids. Added a ElasticSearch.java util file in the utils folder with all relevant helper methods. Getting list of ids through a tag key now goes through ElasticSearch. Also set up a TagResources endpoint.