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

fix configuration parameters #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix configuration parameters #45

wants to merge 1 commit into from

Conversation

tonypiper
Copy link
Collaborator

these were incorrectly defined in #37 and reported in #44. I'm unable to test this at the moment - will check next week.

@michelsalib
Copy link
Owner

Hey @tonypiper is this PR ok ?

@tonypiper
Copy link
Collaborator Author

I've not had a chance to test it yet, and possibly won't this week now - perhaps the author of the original PR could check it?

Tony Piper
Founder and Managing Director
t/f: +44 845 355 0953, m: 07983 444 568, w: antiphonal.net

Antiphonal Ltd
Registered in England and Wales, number 05557992
Registered Office: 27 Old Gloucester Street, London WC1N 3AF

On Wednesday, 3 April 2013 at 13:16, Michel Salib wrote:

Hey @tonypiper (https://github.com/tonypiper) is this PR ok ?


Reply to this email directly or view it on GitHub (#45 (comment)).

@lightglitch
Copy link
Contributor

In my case the worker gets the configurations from correctly from the config:

bcc_resque.vendor_dir   ''
bcc_resque.class    ''
bcc_resque.prefix   ''
bcc_resque.redis.host   ''
bcc_resque.redis.port   ''
bcc_resque.redis.database   ''
bcc_resque.resque.vendor_dir    /var/www/app/../vendor
bcc_resque.resque.class     BCC\ResqueBundle\Resque
bcc_resque.resque.redis.host    20.0.1.76
bcc_resque.resque.redis.port    6379
bcc_resque.resque.redis.database    1 

My problem is that the version in the composer (1.2) doesn't support the REDIS_BACKEND_DB or the PREFIX, that's why it doesn't work.

So you need to change the composer and make the commands compatible with the new version if you want to use this parameters.

@lightglitch
Copy link
Contributor

I have a branch that I use for this: https://github.com/lightglitch/php-resque/tree/branch-1.2

    "repositories": [
        {
            "type": "git",
            "url": "https://github.com/lightglitch/php-resque.git"
        }
    ],
    "require": {
        "chrisboulton/php-resque": "dev-branch-1.2 as 1.2",
    },

@tonypiper and @michelsalib How are you able to use this parameters on your version?

@ruudk
Copy link
Collaborator

ruudk commented Apr 16, 2013

I don't think this PR is correct because it doesn't change anything to the setParameter calls here: https://github.com/michelsalib/BCCResqueBundle/blob/master/DependencyInjection/BCCResqueExtension.php#L28:L32

@tobiaskluge
Copy link
Contributor

The problem is that referenced version of php-resque and php-resque-scheduler not set the database correctly.
@lightglitch correctly fixed this for php-resque, you would need to do something similar for php-resque-scheduler until the dependency of BCCResqueBundle to these libs is updated.

You need to replace the following lines from vendor/chrisboulton/php-resque-scheduler/resque-scheduler

$REDIS_BACKEND = getenv('REDIS_BACKEND');
if(!empty($REDIS_BACKEND)) {
    Resque::setBackend($REDIS_BACKEND);
}

with

$REDIS_BACKEND = getenv('REDIS_BACKEND');
$REDIS_BACKEND_DB = getenv('REDIS_BACKEND_DB');
if(!empty($REDIS_BACKEND)) {
    if (empty($REDIS_BACKEND_DB))
        Resque::setBackend($REDIS_BACKEND);
    else
        Resque::setBackend($REDIS_BACKEND, $REDIS_BACKEND_DB);
}

@lightglitch
Copy link
Contributor

I also have a fork of php-resque-scheduler with that fixed.

@michelsalib
Copy link
Owner

Hi, I just made a cleanup of the PRs, can we handle this issue now ?

@mrbase
Copy link
Contributor

mrbase commented Nov 8, 2013

after the merge of #58 we should be able to test the patch

@mrbase
Copy link
Contributor

mrbase commented Dec 4, 2013

is this PR an issue anymore ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants