Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
juanpujol opened this issue Aug 3, 2016 · 7 comments

Comments

@juanpujol
Copy link

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
Copy link

pulkitsinghal commented Aug 6, 2016

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
Copy link

pulkitsinghal commented Aug 6, 2016

  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
      1. 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

    2. 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. Remove Delete-By-Query plugin elastic/elasticsearch#18516

    2. DBQ plugin replaced by core elastic/elasticsearch-migration#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
Copy link
Author

juanpujol commented Aug 6, 2016

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
Copy link

pulkitsinghal commented Aug 7, 2016

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
Copy link

pulkitsinghal commented Aug 7, 2016

@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 added a commit that referenced this issue Aug 11, 2016
use https://github.com/arielfr/elastic-deletebyquery to bulk delete for destroyAll when working with v2.x of ES
@pulkitsinghal
Copy link

@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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants