RFC: Extract Elastic-specific classes into a separate module

[madmatt]

I'm looking at using this with Algolia, which the module appears to have supported previously but now no longer does. However, if I do I would expect to create a separate module for the Algolia-specific classes. Should we do the same thing with the Elastic classes?

They could both be linked from the module README or similar, with instructions that you need to install at least (or only?) one indexer.

Thoughts welcome, especially from @chrispenny who looks like the most active recent user of the repo!

It'd be a breaking change so would need a new major release, but I don't think it would take too much actual effort to accomplish (but would require a new module be created under the silverstripe org (or someone else could take it on).

[madmatt]

Alternatively, do I simply create a separate Algolia folder and make a PR for the classes in here instead...?

[GuySartorelli]

I agree with this RFC - the fulltextsearch module started off with the intention of being service agnostic and now it's just "that old solr module"
Lets avoid falling to the temptation of making this "that elastic module" by letting it stand alone, and having any elastic-specific code and configuration be its own thing.

That way, if anything does need to change for the stuff that's meant to be agnostic, it'll be a lot easier to distinguish between "we're making this work for elastic" and "we're making this work in an agnostic way, and one of the things it works with is elastic"

[GuySartorelli]

Also as more services get added they should also be their own modules. I don't want a bunch of solr and algolia code sitting around in a website codebase that only integrates with elastic, ya know?

[chrispenny]

I'm working on the upgrade for Silverstripe 5 at the moment. That was already planned to be a new major - so that's probably the best opportunity for us to reframe this module?

[chrispenny]

@madmatt I've had a chat with @GuySartorelli offline.

• Guy has created a new module for the Elastic service classes.
• I'm going to make a best attempt to split these out as part of my efforts for the Silverstripe 5 major.
  • I say "best attempt" because I'm under some time pressure to have this module available for use in an SS5 project, and I hadn't planned for this extra work.
  • I'd very much like to make it happen though.
• For the Algolia services, whether we're able to split the Elastic code out in time or not, it probably makes the most sense for this to be provided through a separate module.

[madmatt]

Thanks for your considered opinions @chrispenny and @GuySartorelli!

I agree with your thoughts exactly Guy, especially around not wanting this to be 'the next fulltextsearch'. Separating out into a separate module shouldn't be hard, otherwise it's probably a sign that it's not decoupled enough.

My project needs to happen in the next couple of months, and as a result will be using CMS 4, but I can look at creating a separate module for Algolia that will eventually be compatible with CMS 4 and 5.

[chrispenny]

@madmatt I was able to spend around an hour on this today, and I made pretty good progress. It does seem like it is decoupled pretty well.

Let me know if there is any minor changes we could make in order to facilitate a second service on the SS4 version - but I think (hope) it will be as easy as providing your own config and classes.

And ditto, my project is starting on the 6th. We have the benefit of a long run time though, which is why we're opting to start with the SS5 beta, rather than upgrade before go-live.

[madmatt]

Hey @chrispenny, @GuySartorelli,

Just wanted to let you both know that I had overlooked an Algolia integration module from Will in my hunt, and given there's already a separate module for Algolia that deals with both indexing and searching I'm likely going to use that for my use case rather than proceeding with rebuilding everything for silverstripe-search-service.

I still agree with the premise of this RFC, but won't have any specific need for it myself anytime soon, so please don't go out of your way purely on my behalf!

Sorry to flip-flop on this one.

[GuySartorelli]

No worries, you started us off in a good direction splitting off the elastic code into its own module, so thank you