php-resque icon indicating copy to clipboard operation
php-resque copied to clipboard

Reduction of external dependencies

Open xelan opened this issue 7 years ago • 6 comments

As part of the discussion in #76, @scones suggested to reconsider and possibly slim down external dependencies. The PR #71 deals already with the Symfony component versions, however the scope of this issue goes beyond that.

In general, I like the idea of reducing unnecessary dependencies so I did some research on the current state.

External dependencies besides predis are at the moment (current master):

  • monolog/monolog ~1.7
  • symfony/console ~2.7|~3.0
  • symfony/yaml ~2.7|~3.0
  • symfony/process ~2.7|~3.0

As far as I can see, the speed test is the only place where symfony/process is currently used. So this dep could be moved to suggest and the speed test could check if it is available.

The core functionality for enqueuing jobs would only require symfony/yaml (for parsing the resque configuration) and monolog/monolog (for logging). Both could be refactored to avoid those external dependencies as well. In the case of the logger this would probably be quite a bit of work. As monolog itself has no dependencies beside psr/log, the question is if that makes sense. For the YAML parsing, it might be beneficial to extract the configuration loading to allow e.g. loading from an array or other sources as well. However, installing the symfony/yaml component by default would probably still be a good idea.

symfony/console would remain as it is required by the commands. The question is whether extracting all of the commands to a separate project is worth the time. According to the Symfony 4.0 and the preliminary master/5.0 upgrade guides, the effort to work with all supported versions of symfony/console is small.

To examine the impacts, benefits and drawbacks of such changes, I'd like to discuss the issue with the most active contributors of the past months, and of course with the project owner.

@mjphaynes @francislavoie @iam-merlin @MaximeMaillet @scones What do you think? Should we consider such a refactoring for the next major release?

xelan avatar Dec 28 '17 19:12 xelan

IMO keep monolog and console. Yaml isn't necessary and we can change the config to be a straight PHP array instead quite easily. I agree with your assessment.

Monolog is quite ubiquitous and overall pretty light so it doesn't really hurt to keep (unless other projects require a different version but we should be flexible here)

francislavoie avatar Dec 28 '17 19:12 francislavoie

Since this suggestion is my work:

  • monolog could be condensed to psr/log, so it can be chosen by the library user.
  • symfony/console has nothing to do with queueing jobs, besides the act of queueing (and other helper tasks). Any funtionality (read: logic) should be moved to this library. The rest can be an own repository for bootstrapping with symfony/console.
  • symfony/yaml is not really necessary for configurstion
  • symfony/process - if its only for speed testing, it surely could be removed

My use case for this library is a docker container with a customized boot-script tho. The symfony/console is useless for me, as is the forking. Monolog just passes through to stdout, i never will use symfony/process, yaml is nice, but not necessary. So my point of view is quite narrow.

update: addendum. When i want to include this project as dependency in other projects in order to spawn jobs, i might have ( and probably will) have dependency conflicts with the versions of symfony & monolog. Since the spawning projects might just be able to spawn Jobs, but not to handle the jobs, those dependencies are quite a show stopper. To make this work, a smaller approach would be needed, that can just create Jobs, but not handle them. Getting rid of the mentioned dependencies would be a huge step in that direction.

scones avatar Dec 29 '17 08:12 scones

+1 for :

  • psr/log
  • symfony/yaml (I would rather prefer env vars... maybe my nodejs background is talking)
  • symfony/process
  • symfony/console

I suggest, following @scones suggestions, to move all commands into another repository. For examle, the start command implementation should be free... it can be reduced to a sample.php file like:

require autoload.php;

// Create worker instance
$worker = new Resque\Worker();
$worker->work();

A worker is nothing more.

On the other hand, more repositories, more complexity...

merlindorin avatar Jan 05 '18 16:01 merlindorin

@mjphaynes Should we create a mjphaynes/php-resque-cli repository to reduce the dependencies and move the command stuff there? Of course this would be a significant BC break and require a major version bump.

Users who are currently using

"require": {
    "mjphaynes/php-resque": "~2.1"
}

would need to change this to

"require": {
    "mjphaynes/php-resque": "~3.0",
    "mjphaynes/php-resque-cli": "~1.0"
}

What do you think?

Thank you, best regards Andreas

xelan avatar Jan 15 '18 14:01 xelan

you can remove mjphaynes/php-resque as dep... mjphaynes/php-resque-cli already have it ^^

merlindorin avatar Jan 16 '18 00:01 merlindorin

Ah, yes of course 😀

xelan avatar Jan 16 '18 10:01 xelan

As I cannot create a new project here, the PHP and Symfony version dump is done with php-resque v3 :partying_face:

xelan avatar Jan 03 '23 12:01 xelan