daycry / cronjob

- Manage Cron jobs in Codeigniter 4
MIT License
64 stars 20 forks source link

Table 'settings' doesn't exist #38

Closed pietos closed 4 months ago

pietos commented 4 months ago

I used your library for a long time, I just updated it to the latest release. After making the minor changes in the Config/CronJob.php file everything seems to work.

Only it looks like your library is now looking for a table called settings, No clue why but this makes the application crash:

PS C:\docker_xampp\htdocs\slurpee_portal> php spark cronjob:list

CodeIgniter v4.5.1 Command Line Tool - Server Time: 2024-06-04 17:32:52 UTC+02:00

[CodeIgniter\Database\Exceptions\DatabaseException] Table 'db_test.settings' doesn't exist at SYSTEMPATH\Database\BaseConnection.php:676

Caused by: [CodeIgniter\Database\Exceptions\DatabaseException] Table 'db_test.settings' doesn't exist at SYSTEMPATH\Database\MySQLi\Connection.php:313

Caused by: [mysqli_sql_exception] Table 'db_test.settings' doesn't exist at SYSTEMPATH\Database\MySQLi\Connection.php:308

Backtrace: 1 SYSTEMPATH\Database\MySQLi\Connection.php:308 mysqli()->query('SELECT * FROM settings WHERE context IS NULL', 0)

2 SYSTEMPATH\Database\BaseConnection.php:722 CodeIgniter\Database\MySQLi\Connection()->execute('SELECT * FROM settings WHERE context IS NULL')

3 SYSTEMPATH\Database\BaseConnection.php:636 CodeIgniter\Database\BaseConnection()->simpleQuery('SELECT * FROM settings WHERE context IS NULL')

4 SYSTEMPATH\Database\BaseBuilder.php:1629 CodeIgniter\Database\BaseConnection()->query('SELECT * FROM settings WHERE context IS NULL', [], false)

5 APPPATH\ThirdParty\vendor\codeigniter4\settings\src\Handlers\DatabaseHandler.php:186 CodeIgniter\Database\BaseBuilder()->get()

6 APPPATH\ThirdParty\vendor\codeigniter4\settings\src\Handlers\DatabaseHandler.php:51 CodeIgniter\Settings\Handlers\DatabaseHandler()->hydrate(null)

7 APPPATH\ThirdParty\vendor\codeigniter4\settings\src\Settings.php:59 CodeIgniter\Settings\Handlers\DatabaseHandler()->has('Config\CronJob', 'filePath', null)

8 APPPATH\ThirdParty\vendor\codeigniter4\settings\src\Helpers\setting_helper.php:25 CodeIgniter\Settings\Settings()->get('CronJob.filePath')

9 APPPATH\ThirdParty\vendor\daycry\cronjob\src\Commands\CronJobCommand.php:69 setting('CronJob.filePath')

10 APPPATH\ThirdParty\vendor\daycry\cronjob\src\Commands\Lister.php:45 Daycry\CronJob\Commands\CronJobCommand()->getSettings()

11 SYSTEMPATH\CLI\Commands.php:70 Daycry\CronJob\Commands\Lister()->run([])

12 SYSTEMPATH\CLI\Console.php:48 CodeIgniter\CLI\Commands()->run('cronjob:list', [])

13 SYSTEMPATH\Boot.php:338 CodeIgniter\CLI\Console()->run()

14 SYSTEMPATH\Boot.php:104 CodeIgniter\Boot::runCommand(Object(CodeIgniter\CLI\Console))

15 ROOTPATH\spark:84 CodeIgniter\Boot::bootSpark(Object(Config\Paths))

pietos commented 4 months ago

When I roll back to the previous version I don't have this issue

daycry commented 4 months ago

Try to run PHP spark migrate --all or change handler to array in Settings config file.

On Tue, 4 Jun 2024, 17:38 Pieter van Os, @.***> wrote:

When I roll back to the previous version I don't have this issue

— Reply to this email directly, view it on GitHub https://github.com/daycry/cronjob/issues/38#issuecomment-2147847291, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZ5DP7BT5QGEYGWLVATFD3ZFXNQPAVCNFSM6AAAAABIYYZYBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBXHA2DOMRZGE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

daycry commented 4 months ago

Copy Settings file from vendor/codeigniter/settings/src/Config/Settings.php to your app Config folder and change the namespace to Config.

Later, you can change hardlers attribute from database to array, but I thing is better run spark migrate command.

pietos commented 4 months ago

We don't use spark for migrations, I see indeed that the migrate --all worked, but it added a new table in our database which I don't want CI4 to use. How come this became an issue in the latest version of your Cronjob library?

pietos commented 4 months ago

Moment I drop the settings table I get back into the same error again: image

pietos commented 4 months ago

Same issue, I would prefer if we can avoid using the CodeIgniter Settings project? https://github.com/codeigniter4/settings

It adds more dependencies which can lead to weird issues over time like the one I am having currently. It's better to stay as clean as possible and close to the original framework.

image

daycry commented 4 months ago

Isn't a issue, settings vendor require a table in your database, but you can try this without this table.

Is it possible that the error will appear when you launch the tests?

If it's in testing, add the following:

use CodeIgniter\Settings\Handlers\ArrayHandler;
use CodeIgniter\Settings\Settings;
/**
     * Sets up the ArrayHandler for faster & easier tests.
     */
    protected function setUp(): void
    {
        parent::setUp();

        $config           = config('Settings');
        $config->handlers = ['array'];
        $this->settings   = new Settings($config);

        Services::injectMock('settings', $this->settings);
    }

But try to add this file in your APP\Config

Settings.php

<?php

namespace Config;

use CodeIgniter\Settings\Handlers\ArrayHandler;
use CodeIgniter\Settings\Handlers\DatabaseHandler;
use CodeIgniter\Config\BaseConfig;

class Settings extends \CodeIgniter\Settings\Config\Settings
{
    /**
     * The available handlers. The alias must
     * match a public class var here with the
     * settings array containing 'class'.
     *
     * @var string[]
     */
    public $handlers = [ 'array' ];

    /**
     * Array handler settings.
     */
    public $array = [
        'class'     => ArrayHandler::class,
        'writeable' => true,
    ];

    /**
     * Database handler settings.
     */
    public $database = [
        'class' => DatabaseHandler::class,
        'table' => 'settings',
        'group' => null,
        'writeable' => true
    ];
}
pietos commented 4 months ago

That works, my extends in the settings.php was wrong. But may I ask why you changed the logic from:

$this->config->maxLogsPerJob

to

setting('CronJob.maxLogsPerJob')

The old way was fine right, now there is another dependency.

daycry commented 4 months ago

I Will do rollback in the Next version for do the code more clean than now.

pietos commented 4 months ago

@daycry awesome, overall the library is amazing.

daycry commented 4 months ago

@daycry awesome, overall the library is amazing.

I have a queue vendor for sync, beanstalkd, Redis and azure Service bus.

If you want try It, feel free.

Althought, I want merge this vendors in one. Queues and cronjobs together.