Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Using non-standard database driver for one model causes the command to fail. #69

Open
nonetallt opened this issue Feb 14, 2024 · 4 comments
Labels
bug Something isn't working Stale

Comments

@nonetallt
Copy link
Contributor

nonetallt commented Feb 14, 2024

What happened?

The Problem

I specified a "clickhouse" connection for one of my models using the laravel clickhouse db driver. Now whenever I run the model:typer command, it results in the following error in the output:

   BadMethodCallException 

  Method PhpClickHouseLaravel\Connection::getDoctrineDriver does not exist.

  at vendor/laravel/framework/src/Illuminate/Macroable/Traits/Macroable.php:112
    108▕      */
    109public function __call($method, $parameters)
    110▕     {
    111if (! static::hasMacro($method)) {
  ➜ 112throw new BadMethodCallException(sprintf(
    113'Method %s::%s does not exist.', static::class, $method
    114▕             ));
    115▕         }
    116▕

  i   Bad Method Call: Did you mean PhpClickHouseLaravel\Connection::getDoctrineColumn() ? 

      �[2m+41 vendor frames �[22m

  42  artisan:35
      Illuminate\Foundation\Console\Kernel::handle()

It seems pretty obvious that the underlying model:show command relies on getDoctrineDriver(), which might not exist for all database drivers.

The Solution

It seems like try-catching for this scenario is not viable, considering the highly generic type of the BadMethodCallException, where catching it might result in unintentionally hiding errors with a different cause.

The first solution that comes to mind would be to implement a feature to ignore certain models (or models with a certain connection type). This should be very straightforward given that the GetModels action already supports exclusion of certain models (the parameters are simply not utilized currently).

Model exclusion could be implemented as either config or command option. Do you happen to have any preference for either approach or would you suggest and entirely different solution to begin with?

Expected Behavior

Model types are generated as normal.

Steps To Reproduce

  1. Install https://github.com/glushkovds/phpclickhouse-laravel
  2. Create a config/database entry for 'clickhouse'
  3. Create a new model. Set protected $connection = 'clickhouse'
  4. Run model:typer command
@nonetallt nonetallt added the bug Something isn't working label Feb 14, 2024
@nonetallt
Copy link
Contributor Author

On an unrelated note, is the packagist auto-update hook broken?

Version 2.2.8 is still not in https://packagist.org/packages/fumeapp/modeltyper where version 2.2.7 is the newest stable release.

@tcampbPPU
Copy link
Member

On an unrelated note, is the packagist auto-update hook broken?

Version 2.2.8 is still not in https://packagist.org/packages/fumeapp/modeltyper where version 2.2.7 is the newest stable release.

I fixed the release 2.2.9 should be latest

@tcampbPPU
Copy link
Member

The Problem

It seems pretty obvious that the underlying model:show command relies on getDoctrineDriver(), which might not exist for all database drivers.
...

This might be more of a model:show laravel command issue as they support which drivers can be used with it. This being a custom driver im not entirely sure. I would be curious if there are any issues out on the framework for this type is issue with unsupported drivers.

My initial thought would be to filter only models with supported drivers as we need Laravel to do its thing before we can try to convert it

Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Stale
Projects
None yet
Development

No branches or pull requests

2 participants