lugensa / scorched

Sunburnt offspring solr client
MIT License
27 stars 19 forks source link

Add missing mlt parameter #55

Closed mlissner closed 3 years ago

mlissner commented 3 years ago

I'm not sure why this option for More Like This is missing. It looks like it's missing from sunburnt as well (I think we vendored sunburnt to fix this back when we used that).

Anyway, this is just a one line change.

I think this is the one thing preventing our py3 upgrade. If I can help release a new version with this change, I'd love to do so.

mlissner commented 3 years ago

If it helps, here's the documentation for Solr for this feature, so you can see it's nothing weird: https://lucene.apache.org/solr/guide/6_6/other-parsers.html#OtherParsers-MoreLikeThisQueryParser

mlissner commented 3 years ago

Any way I can help with this? I'd love to get this merged and do a release if there's any way that's possible.

delijati commented 3 years ago

Sorry for answering so late. Can you add a note to the CHANGES and maybe a test if possible? I would create a new release then.

mlissner commented 3 years ago

We're currently using a fork of this repo so getting a release isn't as important to us as it was before. I'd be happy to do the work to get this PR done, if it looks like this project will be maintained again, but we have additional fixes in our repo that I'd want to pull in here before we could switch back to this. Among them:

Do you think that those two changes would be possible as well — i.e., does it look like you or somebody else will be maintaining this project, or would it be possible for me to become a maintainer? If so, I will create pulls for them as well. We'd much prefer to use this code over our fork and were really bummed when we decided to stop waiting on this. But if it takes months to get one-line patches reviewed, it's pretty hard to stick around.

delijati commented 3 years ago

@lujh @mghh what you guys think of @mlissner proposal? Or give him commit rights or move scorched to a github organization.

lujh commented 3 years ago

@mlissner and @delijati many thanks for the perseverance and I can fully understand, that the situation is not encouraging. The code is used in our projects, but did not need changes. This will eventually change in the next months, so I would like to first try to revive the development. As a start I will take this pull request and integrate it.

delijati commented 3 years ago

@lujh there some more nice things in the fork https://github.com/mlissner/scorched/compare/master...freelawproject:master ;)

mlissner commented 3 years ago

Wonderful news. Thanks @lujh. I'd love to get those other changes merged in too. They're small and they mostly just remove deprecation warnings in py3.

lujh commented 3 years ago

Before I start with the changes, I want to ask, if you would get problems, if I change the test-framework to pytest and change the file layout a little (we use a src subdirectory in all current developments).

mlissner commented 3 years ago

No problems here, nope.

mlissner commented 3 years ago

@lujh thanks for merging this one! Did you have a chance to look at the other two changes in our fork?

Namely:

We're waiting on that to ditch our fork and use this again. The changes are in a diff mentioned upthread.