Skip to content

allow address_layer_filter to be user configurable - #1473

Draft
missinglink wants to merge 1 commit into
masterfrom
address_layer_filter_disabled_via_config
Draft

allow address_layer_filter to be user configurable#1473
missinglink wants to merge 1 commit into
masterfrom
address_layer_filter_disabled_via_config

Conversation

@missinglink

@missinglink missinglink commented Jul 16, 2020

Copy link
Copy Markdown
Member

As discussed in #1274 (comment) the address_layer_filter isn't everyone's cup-o-tea so we should add some ways of configuring or disabling it on a per-installation basis.

As a quick reminder, I'm talking about the bit of code which produces these messages:

Screenshot 2020-07-15 at 14 48 06

The functionality was introduced in #1274 and shouldn't be confused with this similar functionality #1219 and this one #1215 🤷‍♂️


I'm opening this initially as a DRAFT PR so we can discuss it and come up with something which'll work for most people.

My initial commit introduces a config flag api.enable_address_layer_filter which simply enables/disables the filter.
I defaulted it to true so it's backward compatible.

This would probably be helpful for anyone who's running a small-to-medium sized installation of Pelias, because at under a couple million records you're less likely to suffer from the performance pitfalls which originally motivated us adding it.

What it doesn't solve is the case where someone is running a medium-to-large installation of Pelias because performance will likely be pretty bad for one and two letter inputs.

I would consider allowing finer control of the configuration so that it would remain enabled.. but would not be used in the case where a focus.point was provided since #1323 is providing a means of reducing the hit count for those queries.

Also worth noting that having these two different mechanisms for filtering the address layer is confusing, I would like it if we could combine them or delete one of them.

@missinglink

missinglink commented Jul 20, 2020

Copy link
Copy Markdown
Member Author

I'm going to leave this PR open as DRAFT for now so we can use it for testing, however I don't think I'll merge it as-is, I'd considering addressing the following points before merging:

  • Always apply to /v1/search even when config set to false (need to make this obvious to the developer)
  • Optionally only disable address_layer_filter when focus.point is not supplied in autocomplete mode.

@orangejulius

Copy link
Copy Markdown
Member

In order to allow us to close this PR and/or finish off this thought, I'd propose the second option listed above as the smallest amount of functionality that is worth it to merge:

  • only disable address_layer_filter when focus.point is not supplied in autocomplete mode

We'd still have to test it but this would most likely be a positive improvement with little downside.

Otherwise we are looking at adding at least one, if not several config options (for example, allowing individual control of this filter for search and autocomplete).

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

Successfully merging this pull request may close these issues.

2 participants