Skip to content

Revert "Revert ES: update dependencies" #7439

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

Merged
merged 2 commits into from
Oct 8, 2020

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Sep 2, 2020

Reverts #7429

We can merge this together with #7414 since the old endpoint is going to change, better to just get rid of it in the process.

@stsewd stsewd added the Status: blocked Issue is blocked on another issue label Sep 2, 2020
@stsewd stsewd removed the Status: blocked Issue is blocked on another issue label Sep 30, 2020
@stsewd stsewd requested a review from a team September 30, 2020 17:56
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, after some discussion, this seems safe. The change was already reviewed prior, but this is the revert of upgrading our search API results. We backed this out because there were users using the old search API directly. It sounds like most users that we were waiting on have now migrated, so it's safe to upgrade the API again. Because of the way we provide search API results, there is not a clear path for deprecation unfortunately.

I don't have a lot of background here, and am not best to review elasticsearch-dsl changes, so I'm relying on previous review for the technical changes.

@ericholscher
Copy link
Member

ericholscher commented Oct 8, 2020

Yep, I think we're 👍 here. We should be safer going forward with a documented and abstracted search API.

@stsewd stsewd merged commit 7f9778e into master Oct 8, 2020
@stsewd stsewd deleted the revert-7429-revert-7408-update-es-dep branch October 8, 2020 22:57
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.

3 participants