Skip to content
This repository has been archived by the owner on Dec 9, 2019. It is now read-only.

Add support for CRUD operations #22

Closed
wants to merge 34 commits into from
Closed

Add support for CRUD operations #22

wants to merge 34 commits into from

Conversation

duketon
Copy link
Contributor

@duketon duketon commented Feb 12, 2016

Resolves #17

Michael and others added 30 commits December 4, 2015 17:17
Fix config loading for ElasticModule
Add Travis badge and extra config information to README
Add ElasticSearchClient and binding
Parse SearchResponse using Play JSON
Extracted ElasticSearchClient interface and renamed implementation
Collapsed update and insert into upsert
Throw an exception when doc is not a JsObject
Added test which checks proper json parsing
Fix wrong indexTypes configuration
Add ES update operation
Michael and others added 2 commits February 4, 2016 08:55
@duketon duketon mentioned this pull request Feb 12, 2016
* @param exc the execution context
* @return a [[PreparedGet]] instance encapsulating the query
*/
def get(id: String, docType: IndexType)(implicit exc: ExecutionContext): PreparedGet
Copy link
Contributor Author

Choose a reason for hiding this comment

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

id: String

Worries me a bit, since in elastic4s the type of id is Any. @WojciechP @abankowski Do you think we should use Any here for compatibility, or (preferably ;P ) any other ideas?

Copy link
Member

Choose a reason for hiding this comment

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

It's sad, afair they call toString under the hood.

Copy link
Contributor

Choose a reason for hiding this comment

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

Using Strings is fine imo. The scaladocs are slightly broken, though - they require full paths in links, so you should say [[com.evojam.play.elastic4s.client.core.crud.PreparedGet]] instead. Run sbt doc to generate documentation and see the warnings.

.filter(_.isExists)
.map(_.getSourceAsBytes)
.map(Json.parse)
.map(_.as[T])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we be more tolerant with as[T] or assume that if the documents received are not parseable then this is an exceptional situation ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd assume that if someone is aware of inconsistent schema, they should provide a proper Reads instance (e.g. Reads[Either[OldApiEntity, NewApiEntity]]). Throwing an exception is a sane behavior.

@duketon duketon changed the title Add support for Get CRUD operations Add support for CRUD operations Feb 13, 2016
@WojciechP
Copy link
Contributor

The API design has changed - we will leave underlying elastic4s API exposed, so this is no longer valid.

@WojciechP WojciechP closed this Feb 19, 2016
@WojciechP WojciechP deleted the mk-crud branch March 1, 2016 12:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants