search

jina-ai / executors

internal-only
Apache License 2.0
31 stars 12 forks source link

MongoDB: check if embedding before `.pop` #92

Closed cristianmtr closed 3 years ago

cristianmtr commented 3 years ago

https://github.com/jina-ai/executors/blob/d943ed10f282081bb4a49ad00617de71fae81dfd/jinahub/indexers/storage/MongoDBStorage/mongo_handler.py#L87

We have a setup similar to your multireslyrics example. Therefore, the documents indexed by our doc-indexer doesn't have an embedding property. So pop causes an error, because the property doesn't exists. We fixed in internally by a simple if statement:

if 'embedding' in result:
    result.pop('embedding')
sauravgarg540 commented 3 years ago

Hi @cristianmtr , I am interested to work on this, and as per as I understood we need to add an if statement in the file you have linked above. Please let me know if I got it right or if there are some other changes required.

cristianmtr commented 3 years ago

First try to come up with a scenario in a test that shows the problem. Then, yes, the fix is simple, as above.

sauravgarg540 commented 3 years ago

Hi @cristianmtr I have created a PR in which I have added a unit test case that reproduces the error.

cristianmtr commented 3 years ago

Closed by https://github.com/jina-ai/executors/pull/149