Skip to content

Bug: Queue Service with Custom Configuration File #57

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

Open
datamweb opened this issue Mar 29, 2025 · 15 comments · May be fixed by #59 or #58
Open

Bug: Queue Service with Custom Configuration File #57

datamweb opened this issue Mar 29, 2025 · 15 comments · May be fixed by #59 or #58
Labels
bug Something isn't working

Comments

@datamweb
Copy link
Contributor

PHP Version

8.2.0

CodeIgniter4 Version

4.6.0

Queue Package Version

dev:develop

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Queue Driver

database

Queue Configuration

<?php

declare(strict_types=1);

/**
 * This file is part of CodeIgniter Queue.
 *
 * (c) CodeIgniter Foundation <[email protected]>
 *
 * For the full copyright and license information, please view
 * the LICENSE file that was distributed with this source code.
 */

namespace Datamweb\DadAfza\Config;

use CodeIgniter\Queue\Config\Queue as BaseQueue;
use CodeIgniter\Queue\Exceptions\QueueException;
use CodeIgniter\Queue\Handlers\DatabaseHandler;
use CodeIgniter\Queue\Handlers\PredisHandler;
use CodeIgniter\Queue\Handlers\RedisHandler;
use CodeIgniter\Queue\Interfaces\JobInterface;
use CodeIgniter\Queue\Interfaces\QueueInterface;




class Queue extends BaseQueue
{


    /**
     * Database handler config.
     */
    public array $database = [
        'dbGroup'   => 'default',
        'getShared' => true,
        // use skip locked feature to maintain concurrency calls
        // this is not relevant for the SQLite3 database driver
        'skipLocked' => false,
    ];

    /**
     * Default priorities for the queue
     * if different from the "default".
     */
    public array $queueDefaultPriority = [
        'notify' => 'high',
    ];

    /**
     * Valid priorities in the order for the queue,
     * if different from the "default".
     */
    public array $queuePriorities = [
        'notify' => ['high'],
    ];

    /**
     * Your jobs handlers.
     *
     * @var array<string, class-string<JobInterface>>
     */
    public array $jobHandlers = [
        'low_stock'  => \Datamweb\DadAfza\Jobs\LowStockNotification::class,
    ];

}

What happened?

When using the following code:

service('queue', config('Datamweb\DadAfza\Config\Queue'), false)->push('notify', 'low_stock', ['text' => $text]);

I expect everything to work properly, with the configuration data, including the $jobHandlers defined in the file config('Datamweb\DadAfza\Config\Queue'), being executed without issues. However, currently, this is not the case, and I encounter an error.

Steps to Reproduce

Define a custom queue configuration in Datamweb\DadAfza\Config\Queue.

Use the following code to push a job to the queue:

service('queue', config('Datamweb\DadAfza\Config\Queue'), false)->push('notify', 'low_stock', ['text' => $text]);

Observe that the job fails with an error indicating the job handler is not defined in $jobHandlers.

Expected Output

When passing a custom configuration file, such as config('Datamweb\DadAfza\Config\Queue'), I expect the queue service to use this configuration correctly, including all the job handlers ($jobHandlers) defined in it. This should work without issues, and the job 'notify' should be processed correctly according to the handlers specified in the custom configuration.

Anything else?

Instead of using the provided custom configuration, the service falls back to the default configuration, causing the job handlers and other settings to be ignored. This results in the error message indicating that the job name 'notify' is not found in the $jobHandlers array.

@datamweb datamweb added the bug Something isn't working label Mar 29, 2025
@michalsn
Copy link
Member

Please let me know if your custom queue config is properly namespaced and autoloaded.
Can you make dd(config('Datamweb\DadAfza\Config\Queue')) to verify that?

@datamweb
Copy link
Contributor Author

Please let me know if your custom queue config is properly namespaced and autoloaded. Can you make dd(config('Datamweb\DadAfza\Config\Queue')) to verify that?

Image

@michalsn
Copy link
Member

Hmm... I cannot reproduce the problem.

Do you have multiple config files or just one?

@michalsn
Copy link
Member

Can you investigate what is happening in the services file by adding d() and dd()?

// vendor/codeigniter4/queue/src/Config/Services.php
class Services extends BaseService
{
    public static function queue(?QueueConfig $config = null, $getShared = true): QueueInterface
    {
        if ($getShared) {
            return static::getSharedInstance('queue', $config);
        }
d($config);
        /** @var QueueConfig|null $config */
        $config ??= config('Queue');
dd($config);
        return (new Queue($config))->init();
    }
}

If you have only one config file for the queue, you should be able to just set null as a parameter, and autoloader will make the rest for you.

@datamweb
Copy link
Contributor Author

Do you have multiple config files or just one?

@michalsn , thank you!

I have two configuration files, one in path Datamweb\DadAfza\Config\Queue and the other in path Config\Queue, but the way to use them is different.

I’ve now realized that if I remove the file in path Config\Queue, everything works fine, but I don’t want such a limitation.

@michalsn
Copy link
Member

Can you share the reason for two different config files?

If you bring back the version from Config\Queue, can you test how the service is working - just like I showed in my previous post?

I have published the queue config and now have two config files (with different jobHandlers) - still no issues.

@datamweb
Copy link
Contributor Author

Can you share the reason for two different config files?

The reason I need two configuration files is that I’m writing the project as a package, and in the project, I’ve used the configuration in path Datamweb\DadAfza\Config\Queue. Now, to avoid the user having to manually configure the jobs, I’m loading the service with the custom configuration file that exists within the package. This way, the queue service will operate independently of the user’s configuration file(Config\Queue). If the user needs to modify the queue or add entries to the main file, there won’t be any issues.

Config\Queue:

<?php

declare(strict_types=1);

/**
 * This file is part of CodeIgniter Queue.
 *
 * (c) CodeIgniter Foundation <[email protected]>
 *
 * For the full copyright and license information, please view
 * the LICENSE file that was distributed with this source code.
 */

namespace Config;

use CodeIgniter\Queue\Config\Queue as BaseQueue;
use CodeIgniter\Queue\Exceptions\QueueException;
use CodeIgniter\Queue\Handlers\DatabaseHandler;
use CodeIgniter\Queue\Handlers\PredisHandler;
use CodeIgniter\Queue\Handlers\RedisHandler;
use CodeIgniter\Queue\Interfaces\JobInterface;
use CodeIgniter\Queue\Interfaces\QueueInterface;
use Datamweb\Codeigniter4Notifications\Jobs\BaleJob;
use \App\Jobs\Email;

class Queue extends BaseQueue
{
    /**
     * Default handler.
     */
    public string $defaultHandler = 'database';

    /**
     * Available handlers.
     *
     * @var array<string, class-string<QueueInterface>>
     */
    public array $handlers = [
        'database' => DatabaseHandler::class,
        'redis'    => RedisHandler::class,
        'predis'   => PredisHandler::class,
    ];

    /**
     * Database handler config.
     */
    public array $database = [
        'dbGroup'   => 'default',
        'getShared' => true,
        // use skip locked feature to maintain concurrency calls
        // this is not relevant for the SQLite3 database driver
        'skipLocked' => false,
    ];

    /**
     * Redis handler config.
     */
    public array $redis = [
        'host'     => '127.0.0.1',
        'password' => null,
        'port'     => 6379,
        'timeout'  => 0,
        'database' => 0,
        'prefix'   => '',
    ];

    /**
     * Predis handler config.
     */
    public array $predis = [
        'scheme'   => 'tcp',
        'host'     => '127.0.0.1',
        'password' => null,
        'port'     => 6379,
        'timeout'  => 5,
        'database' => 0,
        'prefix'   => '',
    ];

    /**
     * Whether to keep the DONE jobs in the queue.
     */
    public bool $keepDoneJobs = false;

    /**
     * Whether to save failed jobs for later review.
     */
    public bool $keepFailedJobs = true;

    /**
     * Default priorities for the queue
     * if different from the "default".
     */
    public array $queueDefaultPriority = [
        'notify' => 'high',
    ];

    /**
     * Valid priorities in the order for the queue,
     * if different from the "default".
     */
    public array $queuePriorities = [
        'notify' => ['high', 'low'],
    ];

    /**
     * Your jobs handlers.
     *
     * @var array<string, class-string<JobInterface>>
     */
    public array $jobHandlers = [
        'email' => Email::class,
        'bale' => \Datamweb\Codeigniter4Notifications\Jobs\BaleJob::class,
        // 'low_stock'  =>             \Datamweb\DadAfza\Jobs\LowStockNotification::class,
    ];

}

Datamweb\DadAfza\Config\Queue

<?php

declare(strict_types=1);

/**
 * This file is part of CodeIgniter Queue.
 *
 * (c) CodeIgniter Foundation <[email protected]>
 *
 * For the full copyright and license information, please view
 * the LICENSE file that was distributed with this source code.
 */

namespace Datamweb\DadAfza\Config;

use CodeIgniter\Queue\Config\Queue as BaseQueue;
use CodeIgniter\Queue\Exceptions\QueueException;
use CodeIgniter\Queue\Handlers\DatabaseHandler;
use CodeIgniter\Queue\Handlers\PredisHandler;
use CodeIgniter\Queue\Handlers\RedisHandler;
use CodeIgniter\Queue\Interfaces\JobInterface;
use CodeIgniter\Queue\Interfaces\QueueInterface;




class Queue extends BaseQueue
{


    /**
     * Database handler config.
     */
    public array $database = [
        'dbGroup'   => 'default',
        'getShared' => true,
        // use skip locked feature to maintain concurrency calls
        // this is not relevant for the SQLite3 database driver
        'skipLocked' => false,
    ];

    /**
     * Default priorities for the queue
     * if different from the "default".
     */
    public array $queueDefaultPriority = [
        'notify' => 'high',
    ];

    /**
     * Valid priorities in the order for the queue,
     * if different from the "default".
     */
    public array $queuePriorities = [
        'notify' => ['high'],
    ];

    /**
     * Your jobs handlers.
     *
     * @var array<string, class-string<JobInterface>>
     */
    public array $jobHandlers = [
        'low_stock'  => \Datamweb\DadAfza\Jobs\LowStockNotification::class,
    ];

}

@datamweb
Copy link
Contributor Author

Can you investigate what is happening in the services file by adding d() and dd()?

Image

What is clear is that the service successfully receives the passed configuration file(Datamweb\DadAfza\Config\Queue). However, it seems that during development, it was forgotten to actually use this file, and instead, the main configuration file(Config\Queue) is being loaded.

@michalsn
Copy link
Member

Ok, so you get the error not when you push to the queue, but when you use the command queue:work?

The problem is that the worker (and in general all CLI commands) is not designed to choose from two different config files.

I think I would still use single queue config and use Registrar to update it.

@datamweb
Copy link
Contributor Author

Ok, so you get the error not when you push to the queue, but when you use the command queue:work?

Yes.

The problem is that the worker (and in general all CLI commands) is not designed to choose from two different config files.

Yes, exactly!

@michalsn
Copy link
Member

Okay, let me think about it... this would require adding support for additional options (for selecting the config file) for every command.

@datamweb
Copy link
Contributor Author

Honestly, I prefer integration. That means when I use the service with a custom configuration, I expect the workers to automatically detect it.

Maybe using cache to store the name of the custom configuration file and then checking for its existence in the workers could solve this. However, I'm not sure how ideal this implementation would be.

@michalsn
Copy link
Member

No, sorry. This is not a good idea. If you use a custom config file, you should be required to run the worker with that config. The config files may be different, starting from different priorities support and ending with a handler to be used (not only jobHandlers).

If you want to avoid this, you can always use a single config and Registrar to update it from your custom module - and I would recommend that path.

@michalsn michalsn linked a pull request Apr 2, 2025 that will close this issue
5 tasks
@michalsn
Copy link
Member

Although I've prepared changes to fix this, I'm slowly coming to the conclusion that this is probably not a good idea, and I'd be inclined to remove the ability to define a custom config file altogether. We should rather rely on a single file, as is the case with the Tasks package.

A custom config file seems to complicate things unnecessarily... I would recommend avoiding it anyway.

@datamweb
Copy link
Contributor Author

@michalsn thanks for following up on this issue.
To work around the problem in my project, I used Registrar.
In any case, it seems that removing the config file injection and refactoring the code will result in a cleaner and more maintainable structure.

@michalsn michalsn linked a pull request Apr 17, 2025 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants