Return explDict in consistent format #74
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
worleydl
changed the title
worleydl
force-pushed
the
bug/consistent-explDict
branch
from
564c4bb to
3a610f4
Compare
|
Does fixing this also fix o19s/splainer#31 ? If there is a refactoring that you think we should go ahead and get done (beyond what youve done), I'd love to see that happen! |
|
"Find Others" isn't running for solr or elastic on the live site. It is working for Solr under a local build though. I think the functionality can work with this change as Quepid is already doing it, we just need to iron out the kinks in Splainer (maybe another issue?) |
|
awesome…. Yeah.
… On Jan 9, 2020, at 8:05 AM, Daniel Worley ***@***.***> wrote:
"Find Others" isn't running for solr or elastic on the live site. It is working for Solr under a local build though.
I think the functionality can work with this change as Quepid is already doing it, we just need to iron out the kinks in Splainer (maybe another issue?)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#74?email_source=notifications&email_token=AAAFO626S2V3NEIIYJ3PT4DQ44OH3A5CNFSM4KESG2SKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIQHI4I#issuecomment-572552305>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFO6ZDXR5N6HAWDVDCTSLQ44OH3ANCNFSM4KESG2SA>.
_______________________
Eric Pugh | Founder & CEO | OpenSource Connections, LLC | 434.466.1467 | http://www.opensourceconnections.com <http://www.opensourceconnections.com/> | My Free/Busy <http://tinyurl.com/eric-cal>
Co-Author: Apache Solr Enterprise Search Server, 3rd Ed <https://www.packtpub.com/big-data-and-business-intelligence/apache-solr-enterprise-search-server-third-edition-raw>
This e-mail and all contents, including attachments, is considered to be Company Confidential unless explicitly stated otherwise, regardless of whether attachments are marked as such.
|
|
@worleydl I was going to merge and then it struck me, do we need to DRY up this code? why is the exact same query structure show up in two places. |
|
We check in our compiled artifact ( |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When parsing explains from a call to _search, explDict is set to the explanation JSON object (see: https://github.com/o19s/splainer-search/blob/master/factories/esSearcherFactory.js#L218). For the "other" approach, a new object is created with the explanation nested as another property. (see: https://github.com/o19s/splainer-search/blob/master/factories/esSearcherFactory.js#L315)
As far as I can tell nothing is making using of the object structure returned by explainOther and no tests are broken. If we're worried about breaking things we can modify https://github.com/o19s/splainer-search/blob/master/factories/esDocFactory.js#L55 to check for the different structure and return the nested explain if necessary. Ultimately we probably want all methods of explain setting explDict to a consistent format.
This change fixes o19s/quepid#25