Could Drupal\crowdsec\Client::cache be protected instead of private?

What's the use case for that? Why should we allow to decorate this? In fact, the whole class should be internal, and nothing should be altered really, unless there is a compelling reason.

Well, for once it is a service, so it is always a good thing, I've added more info on 💬 ElasticCache Redis issue Active and I think the client should be fair game as it could have other use cases and we have no hooks/OOP ways to change the dsn dynamically, which I need to do right now.

I'm not keen to do that. The Client is a service, but a critical one and it has been deliberately designed to be controlled by the module itself only. Required changes can be done in the module itself, but I don't want to open cache setup up to being altered in any way, because if that fails, it will have consequences that we don't want to support.

How can the dsn calculation be altered then? another protected method for it?

Maybe a different approach would be a weighted plugglable system that would support any backend? so instead of doing a if redis else php files, we could have a plugin manager deciding which cache method to implement in order? maybe Redis and fallback on phpfiles as default but someone could prioritize memcached, couchbase, valkey or a custom calculation for elastic cache for example?

We don't need that level of flexibility. We're implementing required changes such that this service remains fully controlled but still serves all the needs. Then, there is no alter or decoration required.

With Redis, we're now in an awkward situation because the issue at ✨ Support TLS for Predis Needs review is not finalized yet. Therefore, there are 2 steps to take in the meantime:

• Help the other issue getting across the finish line, including the request that it should also support the rediss:// schema.
• Write a patch and apply that to the Crowdsec module. This gets you into a working solution until we're able to provide an official solution.

Unfortunately, we have to wait for the Redis module to decide which way they want to go. I'll post over there as well, hopefully that helps to get it done. Please do the same as well, so that maintainers recognize the importance.

jurgenhaas → closed merge request !13

We don't need that level of flexibility

Well, I've just show you a use case for this. I suspect ✨ Support Memcached for local caching Active is also related to this.

We're implementing required changes such that this service remains fully controlled but still serves all the needs

It does not serve all needs, that's why I try to explain.

Then, there is no alter or decoration required.

I have it decorated with this patch in a real use case. If that's not proof I don't know what is.

You can do reflection, service providers/compiler passes, patching, forking... so you can get the service you need for your project, that's the point of using services... the control you claim is not really there and this should be more flexible, or at the minimum have the possibility of controlling the DSN you eventually connect to, Happy to find ways to work with you to get this more flexible but I need some openness on your side to do so.

Not reopening this because the patch from the MR works for me, but I hope you reconsider.

Attaching the patch as standalone here in case someone else finds it useful.

Those are not use cases, they are only being used to work around bugs that haven't been fixed yet, or features that aren't released yet. And decoration is not the right way to do that.

Fixing the bugs and finalizing features so that they can be published is what's needed. And then, no decoration is needed anymore.

If we allow decoration for a service that we need to have control over, we will end up with support requests when something is doing it wrong. But if we publish proper code, that's never required.

If you still have a use case even under the circumstances that bugs are fixed and features completed, please let us know. The above given examples are not.

So, what are the next steps? Please help with the 2 issues ✨ Support Memcached for local caching Active and ✨ Support TLS for Predis Needs review to get them over the finish line. In the meantime, you're able to patch the code so that you can continue using this in the unfixed environment.

Adding the WIP policy for core.