Elasticsearch implementation #658

paulsullivanjr · 2017-01-22T04:28:36Z

What's in this PR?

This is currently work in progress. But wanted to get an initial PR out.

References

Fixes #60

joshsmith

A couple comments/questions here. I'm completely ignorant to how elastix works so these are get-my-feet-wet questions.

joshsmith · 2017-01-22T06:14:26Z

config/dev.exs

@@ -51,6 +51,10 @@ config :code_corps, :analytics, CodeCorps.Analytics.InMemoryAPI
 config :code_corps, :stripe, Stripe
 config :code_corps, :stripe_env, :dev

+# Configure elasticsearch
+config :code_corps, :elasticsearch_url, "http://0.0.0.0:9200"


Maybe should set these URLs via ENV vars?

That makes sense. I had put these in the config files initially for convenience.

joshsmith · 2017-01-22T06:14:42Z

config/dev.exs

@@ -51,6 +51,10 @@ config :code_corps, :analytics, CodeCorps.Analytics.InMemoryAPI
 config :code_corps, :stripe, Stripe
 config :code_corps, :stripe_env, :dev

+# Configure elasticsearch
+config :code_corps, :elasticsearch_url, "http://0.0.0.0:9200"
+config :code_corps, :elasticsearch_index,  "skills"


Is there a reason we should be setting these indexes in the config?

We shouldn't. I had put these in the config files initially for convenience. But it needs to go away.

paulsullivanjr · 2017-01-25T01:24:32Z

Elastix provides basic functionality for working with elasticsearch. It's a pretty small library (4 modules) and not very long or complex.

npendery · 2017-01-27T21:27:52Z

mix.exs

@@ -4,7 +4,7 @@ defmodule CodeCorps.Mixfile do
  def project do
    [app: :code_corps,
     version: "0.0.1",
-     elixir: "1.3.4",
+     elixir: "1.4.0",


Looks like the circleci is failing because of this, would undo for the PR as the upgrade to 1.4 should probably be in its own PR

Good point, overlooked that. I'll do that this evening.

npendery · 2017-01-27T21:47:25Z

web/helpers/elastic_search_helper.ex

+  def to_map(foo, bar), do: %{ String.to_atom(foo) => bar}
+
+  defp settings_map do
+  %{


Indentation

npendery · 2017-01-27T22:00:30Z

This looks awesome! Great job. One thing I would suggest is maybe making the field_filter extensible for other fields. Maybe have a map as an input with %{field: "type"} and field_filter is generated from this map so if we want to add multiple fields at once we can. Also, not an elasticsearch expert but I believe just by adding more mapping types we could put other models in the same index so maybe have another layer to the input map with type? Doesnt seem like we need it right now but just a thought. This is definitely a good step forward towards a higher level elixir elasticsearch client!

paulsullivanjr · 2017-01-28T02:16:30Z

Thanks for the kind words. I like the ideas you proposed and think they should work. I'll start working on those ideas soon.

joshsmith · 2017-03-10T18:49:41Z

Hey @paulsullivanjr I just wanted to check in on this and see if there's anything we should be doing here?

paulsullivanjr · 2017-03-10T19:10:18Z

@joshsmith I want to discuss what other steps and pieces we'll need to integrate this. I'll catch up with you on slack tonight or tomorrow. I want to get this done so I can get started other other issues.

joshsmith · 2017-03-10T19:45:15Z

Awesome, I will be around to discuss.

joshsmith · 2017-03-16T17:55:46Z

Hey @paulsullivanjr just adding that suggestions comment here for context to this PR:

Adding a skills/search endpoint for searching, handled directly in SkillController
Endpoint point needs to return a collection of Skill structures, need to research how to create from elasticsearch results. The Elasticsearch indexes will need to contain the Skill records from the database including id's.
Keep an ELASTICSEARCH_URL in .env

joshsmith · 2017-03-17T05:51:55Z

web/helpers/elastic_search_helper.ex

@@ -47,7 +47,7 @@ defmodule CodeCorps.ElasticSearchHelper do
  end

  def process_response(%HTTPoison.Response{status_code: 200} = response, type) do
-    response.body["hits"]["hits"] |> Enum.map(fn(x) -> x["_source"][type] end)
+    response.body["hits"]["hits"] |> Enum.map(fn(x) -> x["_source"] end)


What is x here? I think this variable could benefit from a longer name.

joshsmith · 2017-03-17T05:53:02Z

web/controllers/skill_controller.ex

@@ -18,4 +22,9 @@ defmodule CodeCorps.SkillController do
    |> title_filter(params)
    |> limit_filter(params)
  end
+
+  def search(_conn, params) do
+    CodeCorps.ElasticSearchHelper.search(@elasticsearch_url, @elasticsearch_index, @elasticsearch_type, query)


Looks like query is undefined here.

joshsmith · 2017-03-20T02:52:17Z

web/controllers/skill_controller.ex

@@ -23,7 +23,7 @@ defmodule CodeCorps.SkillController do
    |> limit_filter(params)
  end

-  def search(_conn, params) do
+  def search(_conn, %{"query" => query} = params) do


You can remove the = params portion here if you're not using the params as a whole object.

joshsmith · 2017-09-28T06:48:30Z

We could easily get Skills back by simply:

using results = [%{"id" => 1}, %{"id" => 2}] |> Enum.map(&(&1["id"]))
with a where clause: skills = Skill |> where([s], s.id in [1, 2]) |> Repo.all where [1, 2] are the results above

- Removed duplicate entry - Moved some code and a couple small refactors of the helper module - Updated to not pass the document index when creating - cleaning up some unneeded changes - Updated search function - Changed elixir version back to 1.3.4 - Updates per comments, added add_documents() function to helper - added some convenience functions - Removing unnecessary code, eliminated 1.4 warning messages so it's done when we upgrade - Added tests for fuzzy search, updates some map formats - Added match all function - Added search endpoint for elasticsearch, updated to return maps - Added comment about using -- with lists

joshsmith reviewed Jan 22, 2017

View reviewed changes

npendery reviewed Jan 27, 2017

View reviewed changes

joshsmith mentioned this pull request Feb 16, 2017

Add user filter by first, last, username #698

Merged

joshsmith reviewed Mar 17, 2017

View reviewed changes

joshsmith reviewed Mar 20, 2017

View reviewed changes

begedin mentioned this pull request Sep 28, 2017

Onboarding: Skill suggestions #995

Open

joshsmith added the in progress label Oct 5, 2017

paulsullivanjr and others added 2 commits November 25, 2017 20:24

Minor fixes

31cd82c

joshsmith force-pushed the 60-add-elasticsearch branch from 9291262 to 31cd82c Compare November 26, 2017 04:28

begedin force-pushed the develop branch 10 times, most recently from 488d755 to 46dfe17 Compare February 12, 2018 15:14

begedin force-pushed the develop branch 8 times, most recently from e075407 to 6d6cc63 Compare February 12, 2018 16:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Elasticsearch implementation #658

Elasticsearch implementation #658

paulsullivanjr commented Jan 22, 2017 •

edited by code-corps-local bot

Loading

joshsmith left a comment

joshsmith Jan 22, 2017

paulsullivanjr Jan 22, 2017

joshsmith Jan 22, 2017

paulsullivanjr Jan 22, 2017

paulsullivanjr commented Jan 25, 2017

npendery Jan 27, 2017

paulsullivanjr Jan 27, 2017

npendery Jan 27, 2017

npendery commented Jan 27, 2017

paulsullivanjr commented Jan 28, 2017

joshsmith commented Mar 10, 2017

paulsullivanjr commented Mar 10, 2017

joshsmith commented Mar 10, 2017

joshsmith commented Mar 16, 2017

joshsmith Mar 17, 2017

joshsmith Mar 17, 2017

joshsmith Mar 20, 2017

joshsmith commented Sep 28, 2017 •

edited

Loading

Elasticsearch implementation #658

Are you sure you want to change the base?

Elasticsearch implementation #658

Conversation

paulsullivanjr commented Jan 22, 2017 • edited by code-corps-local bot Loading

What's in this PR?

References

joshsmith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulsullivanjr commented Jan 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

npendery commented Jan 27, 2017

paulsullivanjr commented Jan 28, 2017

joshsmith commented Mar 10, 2017

paulsullivanjr commented Mar 10, 2017

joshsmith commented Mar 10, 2017

joshsmith commented Mar 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshsmith commented Sep 28, 2017 • edited Loading

paulsullivanjr commented Jan 22, 2017 •

edited by code-corps-local bot

Loading

joshsmith commented Sep 28, 2017 •

edited

Loading