drush-ops
Handle custom database drivers
The SQL dump command doesn't like if the driver is not one of the core set; this means you can't dump SQL when using a custom driver. Proposing a fix that will only be invoked if the driver is non-core.
Its been a long time, but i think I solved this via a composer.json autoload entry - https://gitlab.com/weitzman/drush-oracle-driver
I think that's a bit of a different situation, TBH. This is a case where the underling db type is supported by Drush, it just doesn't know it can dump from it because the driver name is different from the "base" driver it extends. E.g. in this case it was sqlite - the custom driver needs to be named something else, but it's still sqlite under the hood.
Can't the custom driver create its own class that extends SqlSqlite and override the dumpCmd or dumpProgram() method?. This is what SqlMariaDb does - https://github.com/drush-ops/drush/blob/13.x/src/Sql/SqlMariaDB.php
Except in many cases the custom driver is just providing some additional functionality and doesn't need to provide any custom logic for loading or dumping the db... it's just extending.
Your custom driver can do stuff before or after a call to parent::dumpCmd(), for example. It can be super thin like that mariadb driver is. Sorry, I'm not really following where custom drush db drivers are deficient here.
No apology needed - these things can be hard to suss out. You make a good point that we should support custom drush DB driver logic - and should look for a class matching the pattern in the code to load up. But if your custom drupal DB driver is so thin that it needs not do any custom logic, there's no reason we should require developers to ship a boilerplate Drush db class (and, know they would need to do that) if they are extending a core db type.
I'm afraid we disagree. Extension via OO is a core principle we can and should rely on.
Also, there is no boilerplate in the custom class. It just contains the overridden method(s).
Extension via OO is a core principle we can and should rely on.
I agree with this. But let's say I have a custom driver that exists only to extend sqlite with, say, an entity query condition.
Currently, if I try to drush sql-dump it errors, saying it can't find brads_sqlite driver. I would have to know to ship a Drush db driver class that is empty and only extends the base Sqlite Drush driver to be able to dump.
I'm suggesting that if there is no specific Drush driver found, and the driver extends a driver Drush ships with, use that.
@weitzman I updated this per your feedback... I swallowed anticipated exceptions as suggested but I'm wondering if we shouldn't throw an exception instead of returning NULL here? I think we could provide a better DX in cases where there is no Drush driver found instead of the "can't X on NULL" that you get now if none is resolved.
I'm wondering if we shouldn't throw an exception instead of returning NULL here
Yes, but would need to be a new major version of Drush since its an API change. I'm not sure if any other parts of drush would need to add exception handling. Lets leave as is for now.