Add driver for Couchbase V4 #905
Conversation
|
Hello, Thanks for your contribution, can you first fix the CI by running quality tools on your side please ? PHPMD: PHPSTAN: |
|
Hi I have included all of the config options in the Config class which PHPMD over the edge with all the methords. And you cant have Couchbase V3 and Couchbase V4 extentions loaded at the same time, so If you have any suggestion on how to do so would be helpful. |
|
The only solution I see atm, is to add support of Couchbase v4 for the 9.2 release and deprecate the V3 as of 9.2. |
|
Yes, that is how I am using this driver within my own project, just wanted to push the code back to yourself. |
|
From now unfortunately its the best solution I can offer you. 9.2 will be probably released around end of January 2024. |
|
Yep that is understandable, I was not after a solution. from my side the driver plugins as using CacheManager::addCustomDriver('Couchbasev4', \Framework\Core\Cache\Drivers\Couchbasev4\Driver::class);Also for testing I do have a test for the forking problem which might help, it will need tweeking CacheManager::addCustomDriver('Couchbasev4', Driver::class);
$config = (new Config())
->setUsername('username')
->setPassword('password')
->setBucketName('default')
->setServers(['10.10.10.10'])
->setUseStaticItemCaching(false);
$driver = CacheManager::getInstance('Couchbasev4', $config);
$cache = new Psr16Adapter($driver);
$value = random_int(0, 254);
$cache->set('key1', $value);
$pid = pcntl_fork();
if ($pid == -1) {
die('could not fork');
} else if ($pid) {
pcntl_wait($status);
} else {
exit($cache->get('key1'));
}
$this->assertTrue($value === $status / 256); |
|
Did you found the "custom driver" API friendly to implement, just to be curious ? |
|
I did not find it too bad personaly, but the documentation is lacking, I found reading the innerworkings was best. I had to find out naming convention and namespace requirment of the Item and Config classes, and they need to exist by looking at the following phpfastcache/lib/Phpfastcache/Core/Pool/DriverBaseTrait.php Lines 123 to 130 in c82647a phpfastcache/lib/Phpfastcache/Core/Pool/DriverBaseTrait.php Lines 132 to 139 in c82647a I then had to find that config setters must start with "set" if you are extending ConfigurationOption class for the array loading to work. phpfastcache/lib/Phpfastcache/Config/ConfigurationOption.php Lines 61 to 72 in c82647a From reading to docs the above information is not given, and coders who are lacking skills/time would struggle with impementing a custom driver. IMHO And from a purly personal perspetive and I am not saying your choice is incorrect, but trying to be constructive. CacheManager::addCustomDriver($driverName, $className)
CacheManager::removeCustomDriver($driverName)
CacheManager::addCoreDriverOverride($driverName, $className)
CacheManager::removeCoreDriverOverride($driverName)and gone with CacheManager::addDriver($driverName, $className)
CacheManager::removeDriver($driverName)and let the internals handle the difference between core and custom drivers, IMHO it is the job of phpfastcache to know the difference not implementation code, but this my personal preferance. |
Thanks for the feedback, I will update the WIKI next week to improve readability and understanding of the docs. |
|
Hello, I will wait for Phpstorm stubs to be added: Keep this branch active and allow maintainers to make changes as I will probably make bring changes on it in the next months. At first I wanted to implement it for 9.2, but this will maybe for the V10 (around summer 2024). In any case I need the stubs to be pushed (for PhpStan/PhpMD analysis). |
|
I just have a question @srjlewis By looking at the Couchbase documentation: I feel like the Couchbase extension v4 is completely compatible with v4, are you sure that a new Driver should be made for it, there's no changes we can make to keep the current driver implementation compatible for v3 and v4 ? Knowing that the current implementation is for the v3.2 at least (v3.2 brought support of scopes & collections):
What's your minds on this ? I'd really like to dig this topic deeper with you if you'd like so. Thanks ! |
|
There are slight differences. I do agree it would be great to wrap both in a single driver. Also there is a workaround in v4 diver for a limitation specific to the v4 extension, when used in a forking process. one example is the DocumentNotFoundException v3 |
|
I see, for the exception class its indeed tricky to fool Phpstan 😂 What's the "forking process" exactly ? Another question: Would it be "easier" to make Couchbase_v4 extends Couchbase_v3 Driver/Config/Item class ? So you only override needed properties/methods ? (Better maintainability). |
|
Ok this was my thought process, hopefully it will makes sense. 🤣 I will answaer the question first,
I did not extend the Item class because it is a very small class and would be the same size. The Config class from v4 has the potential of being used in both, but I have not checked the couchbase class compatibility between extentions. use Couchbase\ClusterOptions;
use Couchbase\ThresholdLoggingOptions;
use Couchbase\TransactionsConfiguration;As for the Driver class, I would say v4 as quite a few methods that are different (about 40-50%), so I thought that the changes where big enough to need it's own implementation, and allow potential future changes without making changes to v3 So back to the forking problem here is my bug report with couchbse (https://issues.couchbase.com/browse/PCBC-886) My workaround forces a change in the connection string using posix_getppid(), which forces the internals of the v4 extention to recreate a seperate connection to the parent and not use the persistent from the parent process. I dont mind helping further with this and 🎄Merry Chrismas🎄 |
That makes sense indeed.
I think we can agree on one thing: The v4 is very poorly documented by Couchbase enterprise: Phpstorm stubs are still frozen at v3 despite the v4 extension being released more than a year ago: The official Couchbase-PHP-SDK doc is not even mentioning new changes about exception hierarchy introduced in v4: So, for now, I can't take the risk of removing the v3 until the v4 is not yet properly documented/stabilized. Keep your own implementation using internal Phpfastcache APIs while the v4 is maturing a bit more than it is right now. We'll see next month if things have positively changed (I hope so...), meanwhile thank you for your time and code sharing. Have a nice day srjlewis. |
|
The v4 extension code is now located at (https://github.com/couchbase/couchbase-php-client) Which is a wrapper around the new core code (https://github.com/couchbaselabs/couchbase-cxx-client) And I do agree the docs are bad, as you pointed out, the latest migration dosc are to v3 and there is not even any docs about migration to v4. But I would stress major repos are not even shipping v3 of the extension and are only shipping v4, as the code for the v3 extension is not even compilable for php 8.3 without modification, so anyone using php 8.3 can only use the v4 extension. I do know some repo's have patched their own version of the v3 extension for php 8.3, but I would think that's hit and miss at best between repo's. I hope we can find a balance between the v3 and v4 Thanks |
|
Anyway I need to wait that Couchbase release a Windows build on pecl since my personal coding computer is running on Windows despite my backend are running under a Unix VM. And if you could help me to find reliable v4 stubs, I'd be grateful to have them under my hands 😁 In any case 8.3 has just being released so we still have some time before the v3 get completely deprecated. Judging by the official PHP support timeline there no need to rush for now, right ? |
|
Hi @Geolim4 Stubs are not required now with version v4 as the Couchbase Extention is now a hybrid extention. The only part of the Couchbase v4 namespace that is actually the C Extention which is For you're information on the uptake side of things, I use EL8 adn EL9 distrubutions based from RedHat Linux Enterprise builds. For PHP 8.2 and PHP 8.3 I asked remi to release couchbase v3 extention, but there are changes required within the So within the RedHat Linux world Couchbase V3 extention is well on its way out of the door, as PHP 8.2 and 8.3 v4 is used by default. I will admit I am not sure what the state of the Debian/Ubuntu distros are, on the windows side of things I would of thought they should of release a windows compile by now since v4 has been out for 1.5 years and they are not doing any updates on v3, but I have a feeling they might not release a windows compile or you will need to raise a bug with them (Couchbase). |
|
It's a complete mess. I don't understand anymore what's Couchbase are doing. I'm getting tired to be required to update the whole code of Couchbase every 2 years. Every 2 years they refactor completely the client/library 😖 |
|
I agree very much with that, we use Couchbase quite a lot, I dont mind helping with the maintainace of the couchbase drivers, would you like me to update this driver ready for the changes you are planning in 9.2? |
|
I think I will make it in a separate package as I need to keep the couchbasev3 until the v10, especially since the couchbase v4 requires:
And none of them have Windowsd builds yet :( And due to the extreme difficulty to make them cohabit on the lib with Couchbasev3, the better is to make a I'll add a I will create a new repository for this extension and give you the rights on this one :) |
|
That makes sense for now. |
|
I created the repo: Just a thing: Make sure that posix is loaded in "driverCheck()". I added it in composer requirements, but make sure to make all the checks needed for now. I know it is related to an issue you opened:
But until this issue is fixed keep it as a prerequisite :) |
Yes agreed, I missed that one, I am just used to posix beening there. I think it would be a idea to move/start a new discussion on the new repo. As we need to figure out how the structure for a extention, testing etc. |
|
I disabled the issues on the new repo to keep it on the main repo (here). But I can enable team discussion indeed. Packagist has been created too: I gave you the maintainer rights as well. |
I am fine with ether way
I have just seen that, Thanks |
|
To be honest it's a nice start for something I have in minds for long time now. I always wanted to move specials drivers (Couchbase, Couchdb, Mongodb, etc) to a custom sub-repo with an extension mechanism to lighten the main Library. It may be the start of something I plan for the v10, as a major internal rework of the lib. |
|
I was going to suggest that, but I was being polite and did not want to step on peoples toes and hard work. |
Don't, its something that's been on my mind for years now, but it's a big rework indeed. v10 would be the perfect occasion for that. |
|
I'm closing this one to continue on the discussion of PHPSocialNetwork/couchbasev4-extension 😇 |
2 participants
Proposed changes
I have writen a driver to support Couchbase V4, with a work around for the forking problem within the current offical client (https://issues.couchbase.com/browse/PCBC-886).
Types of changes
What types of changes does your code introduce to Phpfastcache?
Put an
xin the boxes that applyAgreement
I have read the CONTRIBUTING and CODING GUIDELINE docs