Stoppable behavior

[zhuravljov]

Related #87.

The code is simple and anyone can create it inside own project.
But maybe need to create abstract class which will include stopping logics, and cache component should be moved into child?

[TerraSkye]

This wont work when your dealing with EA filecache and 2 different services,
It requires to that the runner and pusher use the same cache instance.

[zhuravljov]

@TerraSkye, you are right, cache must use common storage only.

[cebe]

probably markAsStopped would be a better name.

[zhuravljov]

Yes, it make sense.

[cebe]

documentation is missing, its not really clear to me what this switch is for.

[zhuravljov]

The option allows to turn off status usage that is not supported for AMQP driver.

[samdark]

It should be in the phpdoc.

[cebe]

making this a beforeExec() method instead of a closure could make it easier to extend this class.

[cebe]

This wont work when your dealing with EA filecache and 2 different services,
It requires to that the runner and pusher use the same cache instance.

could be added as a note to the phpdoc.

[samdark]

How about unit tests?

[zhuravljov]

@samdark, tests have been added.

[samdark]

Code looks good to me.

[cebe]

except the naming it looks good to me.

[cebe]

imo it should be yii\queue\behaviors\Stoppable instead of yii\queue\stoppable\Behavior, Behavior as a class name does not make sense to me.

[zhuravljov]

It will be used in a configuration by long name only. As well as closure behavior. Readability shouldn't be impaired.

'queue' => [
    //...
    'as stoppable' => \yii\queue\stoppable\Behavior::class,
    'as closure' => \yii\queue\closure\Behavior::class,
],

I think to make separate behaviors namespace isn't good idea.

[rob006]

It will be used in a configuration by long name only

Why do you think that people uses long names in configuration? I almost always use short names with use statements at the top of the file.

[zhuravljov]

@rob006, I'm not sure. But this make sense for config files, which cover many class definitions from different namespaces.

[zhuravljov]

Anyway, you will resolve name conflicts using namespace aliases. DbConnection, RedisConnection etc. So you can add alias:

use yii\queue\stoppable\Behavior as QueueStoppable;

[rob006]

I talk no about name suffix. Every package class is grouped to namespace by its role, no by class type.

This is extremely inconsistent to be honest. We have yii\queue\serializers for serializers, but doing the same for behaviors is wrong? So now we have at least three conventions for naming behaviors:

• yii\queue\cli\Verbose,
• yii\queue\LogBehavior,
• yii\queue\stoppable\Behavior.

We have Interface suffix for interfaces, Event suffix for events, but name for behavior is simply random.

[zhuravljov]

We have yii\queue\serializers for serializers, but doing the same for behaviors is wrong?

Yes, because serializer implements only one role. It is queue\serializers.

Behavior is queue middleware. And behaviors can solve many different functions. queue\stoppable\Behavior allows to stop a job, queue\closure\Behavior serializes closures, queue\LogBehavior writes logs, etc.

So now we have at least three conventions for naming behaviors

I renamed Verbose to VerboseBehavior, and now we have single naming rule:

yii\queue\<Role><Type>

yii\queue\LogBehavior: Role = queue\Log, Type = Behavior
yii\queue\closure\Behavior: Role = queue\closure\, Type = Behavior
yii\queue\stoppable\Behavior: Role = stoppable\, Type = Behavior
yii\queue\PushEvent: Role = queue\Push, Type = Event
etc.

[rob006]

I renamed Verbose to VerboseBehavior, and now we have single naming rule:

You clearly have two naming conventions:

1. yii\queue\<Role><Type>
2. yii\queue\<role>\<Type>

Anyway, I just hate using general names for specific implementations. It kills readability and productivity. I often use "Search by class name" feature in IDE - it is useless if you name all behaviors as "Behavior". You can't just import such class because you probably will get conflicts and Behavior will be just meaningless if you find it in code.

[zhuravljov]

This is one of common ways of naming:

• yii\<mode>\Application
• yii\<package>\Module
• yii\gii\generators\<entity>\Generator
• Illuminate\Redis\Connections\Connection
• Illuminate\Database\Connection
• <vendor>\<package>\Client

I often use "Search by class name" feature in IDE - it is useless if you name all behaviors as "Behavior".

I'm not sure about all IDEs, but PHPStorm is able to search class usage considering namespace.

[thiagotalma]

This is not a pointless discussion.

In certain discussions there is no right or wrong, just different opinions.
In this particular case, the Yii2 staff could enrich the discussion with their opinions.

In my opinion, the best solution would be to maintain the naming pattern used in Yii2.
After all, it is an extension to a larger project and here is no place to innovate.

cc: @cebe @samdark

[SamMousa]

I don't like this approach at all.

• There is no guarantee they will actually be stopped (a cache is not persistent reliable storage).
• It uses the cache as some kind of quick storage, that is not what it is for.
• If the cache is not reachable for whatever reason, all workers will stop working.
• Several drivers already implement remove() (file, db, beanstalk, redis).

The exceptions here are amqp and gearman.

In the cases where you use amqp / gearman and you have multiple workers, you will need a shared cache.
If you are on the same host then you could use files storage, but in that case you could just as well use the file queue driver since you're depending on the file system anyway.
If you're on different hosts you'll probably end up using a cache driver like redis or memcache; in which case you could just as well use redis for your queue in the first place.

As OP mentioned, implementing it in your own project is simple. Since this (in my opinion) is hardly ever the best, or even a good, solution I think adding this behavior will result in worse decisions by developers.