NotificationsYou must be signed in to change notification settings
Fork370
Star6.6k

Implement daemon mode#351

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

andig wants to merge5 commits intophp-pm:master

base:master

Choose a base branch

fromandig:daemon

Open

Implement daemon mode#351

andig wants to merge5 commits intophp-pm:masterfromandig:daemon

Conversation

Copy link

Contributor

andig commentedFeb 4, 2018

Some code is borrowed fromhttps://github.com/phlib/console-process (LGPL)- need to double-check if that is ok

This was referencedFeb 4, 2018

Reuse for php-pm?phlib/console-process#4

Closed

Add Symfony 4 + PHP-PM in TechEmpower's benchmarksymfony/symfony#25994

Closed

Copy link

Contributor

szepeviktor commentedFeb 5, 2018

Any word about it in the README?

Copy link

Member

We can't use code from a different license if it's just simply copied. If it's inspired or is a construct that is used also everywhere else (like in the documentation, tutorials etc) then we don't need the reference to phlib/console-process as those stuff is not licensable. I personally don't see anything that is defendable although I haven't checked the origin and don't know how hard you copied, if you even did.

Copy link

ContributorAuthor

andig commentedFeb 5, 2018

We can't use code from a different license if it's just simply copied.

Imho they could chose to relicense for us. I've opened an issue at that project. If you have objections please feel free to close but I'm lacking capacity to start over :/

Copy link

chrisminett commentedFeb 6, 2018

Hi,

I'm a maintainer at phlib/console-process

It's fine with us! See comment atphlib/console-process#4

Cheers,
ChrisM

andig force-pushed thedaemon branch 3 times, most recently from665c8fd to4f36839Compare

August 24, 2018 17:22

Copy link

ContributorAuthor

andig commentedAug 24, 2018•
edited
Loading

I've updated and simplified the PR.
~~I would appreciate a second look as the PHP 7 build fails with an unclear error message. Not sure if this is intermittent or real?~~

andig force-pushed thedaemon branch 5 times, most recently from83b15b1 to30d126aCompare

August 27, 2018 16:06

andig added this to the2.0 milestone

Aug 27, 2018

andig added5 commits

September 11, 2018 08:28

Implement daemon mode

7f34516

Move linting first

478e357

Test lowest dependencies

2d675f3

Test worker scaling

55fcf46

Status

e1ed51c

andig force-pushed thedaemon branch from30d126a toe1ed51cCompare

September 11, 2018 06:28

Copy link

Member

marcj commentedOct 22, 2018

What is the main processes dies? Will it automatically restart?

Copy link

ContributorAuthor

andig commentedOct 22, 2018

No, thats nothing I‘ve considered. Should be done via systemctrl daemon setting?

andig added the enhancement label

Mar 7, 2019

Copy link

mowangjuanzi commentedJan 4, 2020

When will it be merged to master? I need it

Copy link

Contributor

acasademont commentedAug 7, 2020

Hi@andig should we move forward with this one?

jkrzefski reviewed

Aug 7, 2020

View reviewed changes

src/Commands/StartCommand.php

		if ($output->isVerbose()) {
		$output->writeln('Daemonized');
		}
		returnnull;

Copy link

Contributor

jkrzefskiAug 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Asthis project supports Symfony 5, the return value of a commandsexecute method must be an integer. While it isnot part of the methods signature in the abstractCommand class, it is stillvalidated and an exception will be thrown for any non-integer return value. So I suggest, you change this toreturn 0; along with the other return statement below.