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

FEAT: Introduce integration with Algolia #933

Merged
merged 7 commits into from
Feb 7, 2024
Merged

Conversation

sumitsarkar
Copy link
Contributor

@sumitsarkar sumitsarkar commented Jan 31, 2024

Description

Introducing Algolia Search Endpoint in Quepid. This would allow users to configure Algolia Endpoint to query a single index.
Multiple indices query is not implemented.
Refer to o19s/splainer-search#144 for the related issue in splainer-search. Once the splainer-search library is released, this PR would be able to add the integration.

Motivation and Context

Closes #932

How Has This Been Tested?

All tests were manually done. I am open to hear from maintainers on what kind of automated tests can be introduced to reduce this overhead.

  1. Used a dev version of splainer-search(FEAT: Enable Algolia integration  splainer-search#145) on Quepid installation.
  2. Created a few Algolia endpoints, and ran searches on an index to list out results, and score them.
  3. Tried cloning a few Algolia endpoints.

Screenshots or GIFs (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds new functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Sumit Sarkar added 3 commits January 31, 2024 10:13
Supports querying single index only.
Multiple indices query is not implemented.
Refer to o19s/splainer-search#144 for the related issue in splainer-search
@epugh
Copy link
Member

epugh commented Jan 31, 2024

Use of lambda is cool! First time I've seen this!

@sumitsarkar
Copy link
Contributor Author

Use of lambda is cool! First time I've seen this!

I don't know if it's a good way to solve this. This is the first time I am writing Ruby code. If there are better alternatives to this, happy to incorporate them in the PR.

Btw, the JS analysis is failing. I suspect it's a false positive as that's how Angular modules are defined. Am I missing something?

Copy link
Member

@epugh epugh left a comment

Choose a reason for hiding this comment

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

Looking great!

// pass pending settings onward to be saved
$scope.pendingSettings.submit = submit;
});

function submit () {
let validateJson = false;
if ( $scope.pendingSettings.searchEngine === 'es' || $scope.pendingSettings.searchEngine === 'os' || $scope.pendingSettings.searchEngine === 'vectara'){
if ( $scope.pendingSettings.searchEngine === 'es' || $scope.pendingSettings.searchEngine === 'os' || $scope.pendingSettings.searchEngine === 'vectara' || $scope.pendingSettings.searchEngine === 'algolia'){
Copy link
Member

Choose a reason for hiding this comment

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

Someday I wish we had a set of capablities that you could consult to make these choices. This code might look more like:

if $capabilitiesSvc.has(JSON_FORMATTED_QUERIES) { 
    validateJson = true;    
}

The $capabilitiesSvc would know what your search engine could do. Or maybe it's more like

if ($scope.pendingSettings.searchEngine.hasCapability(JSON_FORMATTED_QUERIES) {
  validateJson = true;
}

// 'vectara' does not support doc lookup by ID.
let supportLookupById = true;
if (settings && settings.searchEngine === 'vectara'){
supportLookupById = false;
}
else if (settings && settings.searchEngine === 'searchapi'){
supportLookupById = false;
} else if (settings && settings.searchEngine === 'algolia') {
Copy link
Member

Choose a reason for hiding this comment

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

Two questions here! 1) Can we have Algolia do a query by id? We use that to power the Snapshot compare function... It lets you get up to date info on a document from the search engine. For search engines that do not provide it, like vectara and searchapi I am actually looking at dealing with this by just requiring you to store the document fields at snapshot time... But if Algolia did support a lookup by id, that would be nice. I think it would also help in the "misisng documents" screen as well. And then 2) well, this is more a comment, I wish we had the capablities look up for SUPPORT_LOOKUP_BY_ID concept ;-).

Copy link
Contributor Author

@sumitsarkar sumitsarkar Feb 1, 2024

Choose a reason for hiding this comment

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

Sure. I have updated the PR for splainer-search o19s/splainer-search#145.

I have pushed an update for changing this flag as well.

@epugh
Copy link
Member

epugh commented Jan 31, 2024

Use of lambda is cool! First time I've seen this!

I don't know if it's a good way to solve this. This is the first time I am writing Ruby code. If there are better alternatives to this, happy to incorporate them in the PR.

Btw, the JS analysis is failing. I suspect it's a false positive as that's how Angular modules are defined. Am I missing something?

The js tool will "re evaluate" code that gets changed, so the underlying pattern of the failure was already there, and I think we can ignore that one.

@epugh
Copy link
Member

epugh commented Feb 7, 2024

Thanks @sumitsarkar for your work, I put a thank you in https://github.com/o19s/quepid/wiki/Extending-Quepid-for-Your-Search-Engine

@epugh epugh merged commit a233fdd into o19s:main Feb 7, 2024
3 of 4 checks passed
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.

Introduce Algolia Integration
2 participants