search

cliqz-oss / keyvi

Keyvi - a key value index that powers Cliqz search engine. It is an in-memory FST-based data structure highly optimized for size and lookup performance.
https://cliqz.com
Apache License 2.0
179 stars 38 forks source link

Added compiler params support in keyvi cli #217

Closed narekgharibyan closed 7 years ago

narekgharibyan commented 7 years ago

@hendrikmuhs

Fixes #214 Decided do not expose memory_limit param for now and use default to avoid backward compatibility issue later on.

hendrikmuhs commented 7 years ago

@narekgharibyan

A merge conflict is not avoidable as you make use of the API to be changed for 0.2

So if that isn't a super-urgent bug, it would be better to fix it in the 0_2_cleanup branch and get 0.2 ready asap.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 86.698% when pulling a86e84cb3226a8a19dbb4303a8b1702d5b424602 on narekgharibyan:keyvicli_compiler_options into f641487741ed758f61325e68512bd91dd4786f79 on cliqz-oss:master.

narekgharibyan commented 7 years ago

@hendrikmuhs I didn't get what merge conflicts you meant.

hendrikmuhs commented 7 years ago

I mean this:

dictionary = pykeyvi.KeyOnlyDictionaryCompiler(GB, params)

This is deprecated in 0.2.

Not a big deal, but it means I or you have to change it for 0.2 again. That's why I think non-super-urgent features should go into 0_2_cleanup.

narekgharibyan commented 7 years ago

Sure! That's not a problem. Moreover once we merge this PR into master. I'll merge master into 0.2 branch with appropriate fix so the fix will be already there.

hendrikmuhs commented 7 years ago

ok.

LGTM