[16.0][ADD] Connector typesense by kobros-tech · Pull Request #203 · OCA/search-engine

kobros-tech · 2025-04-13T07:20:59Z

based on

#168

OCA-git-bot · 2025-04-13T07:21:04Z

Hi @lmignon,
some modules you are maintaining are being modified, check this out!

kobros-tech · 2025-04-21T17:05:19Z

@lmignon
@wlin-kencove
@dnplkndll

Can we have this module reviewed?

kobros-tech · 2025-04-22T11:49:07Z

@lmignon
@sbidoul

I am planning to benefit from shopinvader serializing, shall we cooperate to produce a module relying on it here or I would rely on my effort alone?

I only need your repo to be available here or create a specific one in OCA?

lmignon

Some little comments but otherwise looks good.

lmignon · 2025-04-23T06:43:58Z

@lmignon @sbidoul

I am planning to benefit from shopinvader serializing, shall we cooperate to produce a module relying on it here or I would rely on my effort alone?

I only need your repo to be available here or create a specific one in OCA?

@kobros-tech You could also contribute on the shopinvader repo. Shopinvader contributors are all OCA members. All these modules could have been developed under the OCA and follow the same rules as those put in place by this organization. For the time being, however, they are managed in an independent repo for historical reasons, but also because they are designed to meet a specific context of use. It should be noted that all low-level modules are in the OCA, and are the result of the major architectural work we have carried out and continue to carry out for Shopinvader: connector_searchengine, fs_, fastapi and pydantic integrations in Odoo, (base_rest, storage_) etc... This situation is not set in stone and is open to discussion.

kobros-tech · 2025-04-23T09:48:15Z

all right, I will review changes

For me, I am thinking of high mobility connection instead of creating a new module for each new serialised field.

ir.exports with serialozers module have a solution but need some improvement and interactive dialogs, I am working on that in a new module here and PR referring to this one.

I could only benefit from shopinvader by documenting each field but later on.

Thanks @lmignon

codeagencybe · 2025-05-06T10:08:22Z

all right, I will review changes

For me, I am thinking of high mobility connection instead of creating a new module for each new serialised field.

ir.exports with serialozers module have a solution but need some improvement and interactive dialogs, I am working on that in a new module here and PR referring to this one.

I could only benefit from shopinvader by documenting each field but later on.

Thanks @lmignon

This module can be interesting for odoo website/ecommerce as well, not just shopinvader. Even for other headless use cases to consume Typesense for search + filtering in a shop.

I was about to consider to create a module for meillisearch. Very similar to Typesense.

Is there any final decision where the module is going? Is there also a v18 migration yet or planned?

kobros-tech · 2025-05-06T13:32:54Z

@codeagencybe

thanks, if you are an OCA member you could review the module as we always do as OCA members for the modules that are interesting.

As long as you like it and wishing to have a 18.0 migration, you can help to get this one merged first and then anybody will work on migration to 18.0 like me as planned.

I am not interested in shopinader after creating this #204 module which saves me +50 not needed modules!

I know Fabio that you use shopinvader, but I am not using it and to have one module to serialize all search engine I wish you succeed with this idea.

Good Luck Fabio and I wish you have recovered from Covid.

thienvh332

Thanks for the update! I’ve reviewed the changes once again and tested the functionality, everything is working smoothly on my end 👍

I found a few places where we could make some updates.
Nothing blocking, just some nitpicking 😉

thienvh332 · 2025-05-13T10:50:17Z

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<!-- Copyright 2019 ACSONE SA/NV


Suggested change

<!-- Copyright 2019 ACSONE SA/NV

<!-- Copyright 2024 Derico

in fact none of them deserve the copy right except they had the first trial, but Kencove is doing the action on it now.

thienvh332 · 2025-05-13T11:01:58Z

+    def delete(self, binding_ids) -> None:
+        ts = self._ts_client
+        for binding_id in binding_ids:
+            try:
+                ts.collections[self._index_name].documents[f"{binding_id}"].delete()
+            except typesense.exceptions.ObjectNotFound as e:
+                _logger.info(
+                    f"Could not find a document with id: '{binding_id}'... \n{e}"
+                )


Each call to ts.collections[self._index_name].documents[f"{binding_id}"].delete() sends a separate request.
This approach may be inefficient when deleting a large number of documents.

Proposal: Use delete-by-query instead for better performance.

Example:

ts.collections[self._index_name].documents.delete(filter=f"id:{binding_ids}")

ok, I was not relying on id filter while building up the module, but now as it is functioning well and has its own serializer to convert id value into string, I may use it.

calling filter method how can be faster that calling the exact document by its id directly, I am not sure.

I'm thinking about the issue of deleting multiple records at once.
For example, deleting 100 records: with the initial approach, the server would send 100 separate HTTP requests. Do you think we need to address this issue?

IMO I there is a more efficient option we should implement it.
https://typesense.org/docs/28.0/api/documents.html#delete-a-single-document
I only see the record and query options though?

Hi @dnplkndll ,
I also referred to that documentation. You can scroll down a bit to the 'delete-by-query' section and you'll see a 'TIP' part.

Actually, this doesn't block anything, it's just a suggestion for improvement.

I can understand you mean we make just a single API call to delete all recordsets we have instead of looping through them and making API call for each?

It is a fantastic idea, thanks!

kobros-tech · 2025-05-14T06:55:39Z

try to see what happens, if the issue arises raise it against the base module as all methods here are managed by cron and queued jobs.

sebastienbeau · 2025-06-04T10:25:37Z

+    serializer_type = fields.Selection(selection_add=[("typesense", "Typesense")])
+
+    def _get_serializer(self):
+        if self.serializer_type == "typesense":
+            return TSJsonifySerializer()
+        else:
+            return super()._get_serializer()


It do not make sense to add a serializer here.
Serializer are not "linked" to a kind of search engine, but more on a kind of usage "shopinvader" for example.

shopinvader is doing the same by the way in its custom modules

but I am not following shopinvader as it looks old technique and hard code serialzing but I do dynamic serialzing.

it will better to add a "dynamic serialer" in a separated module, because the serializer is not related to typesense (it can be used for other search engine

shopinvader is doing the same by the way in its custom modules

but I am not following shopinvader as it looks old technique and hard code serialzing but I do dynamic serialzing.

@kobros-tech

old technique 🤔 We follow a different pattern but not an old technique since in v8 we started with dynamic serializing using the base_jsonify... The uses of pre-defined models is motivated by:

the need to control the format and the content exported to the search engine to avoid any leak of information. * the need to provide the documentation related to the data exported

define a strict contract between Odoo and ES (If the internal model of odoo evolve when migrating from one version to another, we are sure to not break the website since the mapping is explicit and our unittest will fails)

...

There are no old and new or right and wrong approaches. It all depends on individual needs and sensitivities. With this in mind, the architecture of these modules has been designed to allow easy adaptation to individual needs.

I am sorry, I can't compare one single module to a project :)

sebastienbeau · 2025-06-04T10:47:57Z

Hi sorry for the delay for the review.

Thanks a lot for helping on this PR ! And thanks for your interest in the project

This week I want to focus on this PR in order to get it merged ! So I can help to fix it and test it.
Are you able for fixing stuff this days? If not I can do it (I am at the OCA spain code sprint, and so available for doing it).

Instead of squashing all in one commit can you do one commit for work done by @MrTango and one for the work done by you ? (it's always better for historic purpose)

Regarding the fact of having generic module for the serializer for the search engine. I am not closed on this idea.
On shopinvader we decide to move on Pydantic schema as default exporter. And so, most of shopinvader related schema module are already split on small module so right now you do not need to install all the shopinvader stack for having the schema, you just have to install related schema and search engine module.
But for sure there is a "lot" of module for the search engine part (as it's small module that just extend an existing schema).

kobros-tech · 2025-06-04T12:59:35Z

I don't think it is following OCA rules

only merged commits can't be squashed or the commits that can introduce fix to issues in many branches

but the old commits were not fixes but they were just trials

sebastienbeau · 2025-06-04T14:06:08Z

For OCA there is no issue with having 2 commits.
Right now there is only one commit where @MrTango is the author so you work is "invisible" and if somebody do a rebase you will not appear anymore in git history.
So this is why I think it's clearer to keep at least 2 commits one with the work of @MrTango and one with your work.

I can do it if you want, I do not want to add you extra work, I just want to add your credit in git history ;)

kobros-tech · 2025-06-04T14:10:46Z

For OCA there is no issue with having 2 commits. Right now there is only one commit where @MrTango is the author so you work is "invisible" and if somebody do a rebase you will not appear anymore in git history. So this is why I think it's clearer to keep at least 2 commits one with the work of @MrTango and one with your work.

I can do it if you want, I do not want to add you extra work, I just want to add your credit in git history ;)

thanks very much

I respect this of you
In fact I preferred to ignore myself if someone complains but as long as you don't mind I also don't mind

sebastienbeau · 2025-06-04T15:36:16Z

I prefer to split in two commit, so it's easier to review, also it will make easier for @MrTango to review the change.
I am going to do a PR, and also to propose some extra refactoring.

sebastienbeau · 2025-06-04T15:57:19Z

Note: as a best practice for the review, the best should have been to open a PR on the branch of @MrTango so we can review it, and merge it in his PR. But don't worry it's just a tips for the next time.

Any case thank for you contribution and your time. I am starting some review / refactoring here : #210, I should have something ready tomorrow

MrTango · 2025-06-04T16:47:02Z

Good to see progress here. Unfortunately i can't spend time this and next week. But I'll have a look at the situation in a couple of weeks and plan to sprint on it latest in Liege.

kobros-tech · 2025-06-04T18:43:34Z

Note: as a best practice for the review, the best should have been to open a PR on the branch of @MrTango so we can review it, and merge it in his PR. But don't worry it's just a tips for the next time.

Any case thank for you contribution and your time. I am starting some review / refactoring here : #210, I should have something ready tomorrow

I think before I start this PR I told them in the first original one, for me I don't care who does what as much as the work be good and everybody's work is remembered.

For opening a new PR I am used to not just with others but with myself, sometimes I don't trust one migration and proposes the other and discuss until we merge the best accepted version.

I put into account others yes, as you saw the commit was not having my name.

As long as I see interest and activity I am happy too.

sebastienbeau · 2025-06-06T18:15:42Z

Replaced by : #210

kobros-tech force-pushed the connector_typesense branch 7 times, most recently from 1ddc5f3 to 27ce831 Compare April 16, 2025 13:57

kobros-tech force-pushed the connector_typesense branch 10 times, most recently from 4424191 to 5e79387 Compare April 21, 2025 16:52

kobros-tech force-pushed the connector_typesense branch from 5e79387 to 3e415d8 Compare April 22, 2025 13:29

kobros-tech mentioned this pull request Apr 23, 2025

[16.0][ADD] typesense_ir_exports #204

Closed

lmignon reviewed Apr 23, 2025

View reviewed changes

Comment thread connector_typesense/models/se_backend.py

lmignon requested changes Apr 23, 2025

View reviewed changes

Comment thread connector_typesense/models/se_index.py Outdated

Comment thread connector_typesense/tests/test_connector_typesense.py

kobros-tech force-pushed the connector_typesense branch 2 times, most recently from ae63122 to ba7b886 Compare April 23, 2025 17:06

kobros-tech force-pushed the connector_typesense branch 2 times, most recently from a279875 to 50567db Compare May 4, 2025 00:01

thienvh332 reviewed May 13, 2025

View reviewed changes

kobros-tech force-pushed the connector_typesense branch from 50567db to 147f7f5 Compare May 13, 2025 12:33

[16.0][Add] connector_typesense: a connector to typsense search engine

6e59027

kobros-tech force-pushed the connector_typesense branch from 147f7f5 to 6e59027 Compare May 15, 2025 23:32

sebastienbeau added this to the 16.0 milestone Jun 3, 2025

sebastienbeau requested changes Jun 4, 2025

View reviewed changes

sebastienbeau mentioned this pull request Jun 4, 2025

[16.0] Connector typesense #210

Merged

1 task

sebastienbeau closed this Jun 6, 2025

		@@ -0,0 +1,14 @@
		<?xml version="1.0" encoding="utf-8" ?>
		<!-- Copyright 2019 ACSONE SA/NV

Uh oh!

Conversation

kobros-tech commented Apr 13, 2025

Uh oh!

OCA-git-bot commented Apr 13, 2025

Uh oh!

kobros-tech commented Apr 21, 2025

Uh oh!

kobros-tech commented Apr 22, 2025

Uh oh!

Uh oh!

lmignon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lmignon commented Apr 23, 2025

Uh oh!

kobros-tech commented Apr 23, 2025

Uh oh!

codeagencybe commented May 6, 2025

Uh oh!

kobros-tech commented May 6, 2025

Uh oh!

thienvh332 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kobros-tech commented May 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmignon Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sebastienbeau commented Jun 4, 2025

Uh oh!

kobros-tech commented Jun 4, 2025

Uh oh!

sebastienbeau commented Jun 4, 2025

Uh oh!

kobros-tech commented Jun 4, 2025

Uh oh!

sebastienbeau commented Jun 4, 2025

Uh oh!

sebastienbeau commented Jun 4, 2025

Uh oh!

MrTango commented Jun 4, 2025

Uh oh!

kobros-tech commented Jun 4, 2025

lmignon Jun 5, 2025 •

edited

Loading