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

Remove cutoff_frequency, deprecated in es7.3.0, forbidden in es8 #134

Merged

Conversation

michaelkirk
Copy link
Contributor

@michaelkirk michaelkirk commented Jul 21, 2023

From https://www.elastic.co/guide/en/elasticsearch/reference/8.8/migrating-8.0.html

The cutoff_frequency parameter has been removed from the match and
multi_match query.

Details
The cutoff_frequency parameter, deprecated in 7.x, has been removed
in 8.0 from match and multi_match queries. The same functionality
can be achieved without any configuration provided that the total
number of hits is not tracked.

Impact
Discontinue use of the cutoff_frequency parameter. Search requests
containing this parameter in a match or multi_match query will
return an error.

Note in the above "...provided that the total number of hits is not tracked".

track_total_hits does not appear in the pelias codebases, so we shouldn't have any issues there.


Here's what actually got changed 👏

I deleted all usage of cutoff_frequency and updated the tests.


Here's how others can test the changes 👀

I believe this is only used by pelias-api, so I've tested it via:

  • cd pelias/api && npm test (with my fork that uses this branch.)
  • cd pelias/docker/projects/north-america && ../../pelias test run (with my fork that uses this branch.)

I haven't tested it against the planet or other builds.

Note: The north-america tests aren't all passing, but the ones that are failing are exactly the same as when I run from current master. I'm assuming (🤞) that the failures reflect changes in the input data since the tests were updated, as hypothesized over here.

Aggregate test results
Pass: 233
Improvements: 18
Expected Failures: 28
Placeholders: 0
Regressions: 84
Total tests: 363
Took 7774ms
Test success rate 76.86%

north-america integration test output before
north-america test output after

closes #118

From https://www.elastic.co/guide/en/elasticsearch/reference/8.8/migrating-8.0.html

    The cutoff_frequency parameter has been removed from the match and
    multi_match query.

    Details
    The cutoff_frequency parameter, deprecated in 7.x, has been removed
    in 8.0 from match and multi_match queries. The same functionality
    can be achieved without any configuration provided that the total
    number of hits is not tracked.

    Impact
    Discontinue use of the cutoff_frequency parameter. Search requests
    containing this parameter in a match or multi_match query will
    return an error.

Note in the above "...provided that the total number of hits is not
tracked".

`track_total_hits` does not appear in the pelias codebases, so we
shouldn't have any issues there.
@missinglink
Copy link
Member

@michaelkirk is this still a requirement for es8, seems to be running ok without it?

@michaelkirk
Copy link
Contributor Author

Things are running fine for me without it. Be aware that I'm only using the autocomplete/ and place/ endpoints.

TBH I don't remember the context of this PR — if I actually hit some error or if I was simply following the migration guide.

@missinglink missinglink merged commit 28a73ba into pelias:master Apr 18, 2024
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.

Deprecate cutoff_frequency for ES7
2 participants