loopbackio / loopback-connector-elastic-search

Strongloop Loopback connector for Elasticsearch
MIT License
79 stars 56 forks source link

Getting error when trying to delete: self.db.deleteByQuery is not a function #45

Closed juanpujol closed 8 years ago

juanpujol commented 8 years ago

I'm using ElasticSarch 2.1.1 and loopback-connector-elastic-search 1.0.5 and every time I try to delete a record by id I get this error:

{  "error": {
    "name": "TypeError",
    "status": 500,
    "message": "self.db.deleteByQuery is not a function",
    "stack": "TypeError: self.db.deleteByQuery is not a function\n    at destroyAll (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-connector-es/lib/esConnector.js:796:13)\n    at doDelete (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/dao.js:1936:19)\n    at /Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/dao.js:1910:9\n    at doNotify (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:98:49)\n    at doNotify (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:98:49)\n    at doNotify (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:98:49)\n    at doNotify (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:98:49)\n    at Function.ObserverMixin._notifyBaseObservers (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:121:5)\n    at Function.ObserverMixin.notifyObserversOf (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:96:8)\n    at Function.ObserverMixin._notifyBaseObservers (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:119:15)\n    at Function.ObserverMixin.notifyObserversOf (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:96:8)\n    at Function.ObserverMixin._notifyBaseObservers (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:119:15)\n    at Function.ObserverMixin.notifyObserversOf (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:96:8)\n    at Function.ObserverMixin._notifyBaseObservers (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:119:15)\n    at Function.ObserverMixin.notifyObserversOf (/Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/observer.js:96:8)\n    at /Users/juan/Documents/Projects/bkn-buildings-api/node_modules/loopback-datasource-juggler/lib/dao.js:1908:13"
  }
}

An important issue since I'm not able to remove anything.

Thank you.

pulkitsinghal commented 8 years ago

This is a valid bug, I verified by running tests against v2.x, thanks to @harshadyeola for setting up easy to use docker instructions

Ah, dang! Its deprecated in 1.5.3 according to docs

screen shot 2016-08-06 at 12 48 49 am

pulkitsinghal commented 8 years ago
  1. Latest stable version of ES is 2.3.4 as of now and that may be made to work according to this comment:

    delete-by-query has been implemented using bulk, and moved into a plugin

    1. If we want to use the plugin approach, then the users with ES need to use the following for v2.3 etc: https://github.com/elastic/elasticsearch/tree/2.3/plugins/delete-by-query
    2. From the connector code persepective, we need to update the connector to use https://github.com/dermidgen/elasticsearch-deletebyquery
     which is a:
    
     > `Elasticsearch-js client extension for the Delete-by-query plugin`
    1. If we want to use the scroll/scan approach, someone was nice enough to share that too! https://github.com/arielfr/elastic-deletebyquery
  2. But I think the master version of ES tells a different story for the future completely:

    The Delete-By-Query plugin has been removed in favor of a new Delete By Query API implementation in core. It now supports throttling, retries and cancellation but no longer supports timeouts. Instead use the cancel API to cancel deletes that run too long.

    1. https://github.com/elastic/elasticsearch/pull/18516
    2. https://github.com/elastic/elasticsearch-migration/issues/61
    3. And its back again in reference/master docs! Huh? screen shot 2016-08-06 at 1 19 57 am
    4. Old(1.5.3) versus SuperNew(master/5.x)
      #old
      DELETE twitter/tweet/_query
      {
          "query" : {
              "term" : { "user" : "kimchy" }
          }
      }
    
      #super new
      POST twitter/_delete_by_query
      {
        "query": { 
          "match": {
            "message": "some message"
          }
        }
      }
juanpujol commented 8 years ago

I added a conditional to the destroyAll method so it will work with the native delete using just id from ElasticSearch v2.x.

https://gist.github.com/juanpujol/3a3ad4aa8a60b649f632127bd5814d2f#file-esconnector_destroyall-js-L27

It should work as long the apiVersion setting is set to 2.x.

This works for me but I don't know if should go on a PR.

pulkitsinghal commented 8 years ago

BTW, it only seems appropriate that I thank both:

  1. @arielfr for https://github.com/arielfr/elastic-deletebyquery,
  2. and @dermidgen https://github.com/dermidgen/elasticsearch-deletebyquery

for their separate and novel approach ... without them my research into a possible resolution would not get as far.

pulkitsinghal commented 8 years ago

@juanpujol - I like what you have and I do think it would have made a valid PR, even if there may have been a little bit of back & forth.

Right now I'm trying to use https://github.com/arielfr/elastic-deletebyquery to make it work, a bit stuck but let's hope I figure it out as it would be a more complete solution. Also am worried about blocking code but lets see.

pulkitsinghal commented 8 years ago

@juanpujol - it is done and released in 1.0.7 of this connector, please take it for a spin and reopen this issue if it is not fixed.

Out of the many approaches, I chose to use https://github.com/arielfr/elastic-deletebyquery to make it work which means that scam/scrol is used in tandem with bulk queries to ES v2.x

juanpujol commented 8 years ago

@pulkitsinghal sorry it took me so long. I tested and it works, thank you!