Closed AlexeyRaga closed 3 years ago
@bitemyapp Sorry, I did not realise that Travis is using a different way of provisioning the ES and that there was an example project.
I have fixed them both and now the build is green, please have another look.
@AlexeyRaga I'm interested in supporting this and updating Bloodhound to ES 7. I've asked @JoseD92 to combine his ES 7 update effort with what you've done. So even if we don't merge this specific PR in the end, it's likely the final PR will reflect the good work you've done here.
@bitemyapp Where I can see these changes? We are currently maintaining our own fork of bloodhound
and it would be useful to see what I missed and probably get these changes too until the official release is out.
With @JoseD92 PR merged, what holds us back from having ES7 support in master
and on Hackage?
@bitemyapp @AlexeyRaga any update on this ?
that would be awesome, as well as an update on katip-elasticsearch. I can help but would like some confirmation this works first.
cc @wraithm
I've tried to use this with katip-elasticsearch and this update breaks it. I wonder where it's best to break the interface bloohound vs katip (I don't know enough about either to have an opinion)
cc @MichaelXavier https://github.com/bitemyapp/bloodhound/pull/266#issuecomment-717930767
@wraithm I've addressed the style nits you raised.
It seems that the original author is no longer in a position to maintain this particular PR. If there are any other changes that need to be made, I am happy to have a go - but I will need to re-open this under a fork I control. (For example, it seems that the disabled test could be re-enabled).
Great! Thanks. I'll get to this later today.
@MichaelXavier Ping, do you have any opinions about katip-elasticsearch and this PR? I've reviewed this, and I'm pretty sure this is good to go.
Changes
docker compose
to run ES 7.4.1MappingType
Bloodhound
)Note that one tests is currently disabled due to a bug that I have found in the ES itself: https://github.com/elastic/elasticsearch/issues/48670 The bug hits rather rare use case, and
bloodhound
is not at fault so I think it shouldn't be a show stopper for moving forward.