Consumer timeout wait #36

constantable · 2020-10-14T13:34:53Z

Allows to set timeout_wait flag for consumer
Inpired by the PR php-amqplib#559

andreea-anamaria · 2020-10-15T09:24:28Z

composer.json

@@ -1,5 +1,5 @@
 {
-    "name": "emag-tech-labs/rabbitmq-bundle",
+    "name": "constantable/rabbitmq-bundle",


Please revert this.

andreea-anamaria · 2020-10-15T09:25:06Z

README.md

-[![Latest Version](http://img.shields.io/packagist/v/emag-tech-labs/rabbitmq-bundle.svg?style=flat-square)](https://github.com/eMAGTechLabs/RabbitMqBundle/releases)
-[![Build Status](https://travis-ci.org/eMAGTechLabs/RabbitMqBundle.svg?branch=master)](https://travis-ci.org/eMAGTechLabs/RabbitMqBundle)
+[![Latest Version](http://img.shields.io/packagist/v/constantable/rabbitmq-bundle.svg?style=flat-square)](https://github.com/constantable/RabbitMqBundle/releases)
+[![Build Status](https://travis-ci.org/constantable/RabbitMqBundle.svg?branch=master)](https://travis-ci.org/econstantable/RabbitMqBundle)


Please revert this.

andreea-anamaria · 2020-10-15T09:25:23Z

README.md


 ```bash
-$ composer require emag-tech-labs/rabbitmq-bundle
+$ composer require constantable/rabbitmq-bundle


Please revert this.

andreea-anamaria · 2020-10-15T09:25:32Z

README.md

 {
    "require": {
-        "emag-tech-labs/rabbitmq-bundle": "^2.0",
+        "constantable/rabbitmq-bundle": "^2.0",


Please revert this.

andreea-anamaria · 2020-10-15T09:34:56Z

Tests/RabbitMq/ConsumerTest.php

        $consumer->consume(1);
    }
+
+    public function testTimeoutWait()


Please rerun this test and check the failing reason:
1) OldSound\RabbitMqBundle\Tests\RabbitMq\ConsumerTest::testTimeoutWait Expectation failed for method name is "wait" when invoked 2 time(s) Parameter 2 for invocation PhpAmqpLib\Channel\AbstractChannel::wait(null, false, 29) does not match expected value. Failed asserting that 29 matches expected 30. /home/travis/build/eMAGTechLabs/RabbitMqBundle/RabbitMq/Consumer.php:95 /home/travis/build/eMAGTechLabs/RabbitMqBundle/Tests/RabbitMq/ConsumerTest.php:398

Check also:
3) OldSound\RabbitMqBundle\Tests\RabbitMq\DynamicConsumerTest::testTimeoutWait Expectation failed for method name is "wait" when invoked 2 time(s) Parameter 2 for invocation PhpAmqpLib\Channel\AbstractChannel::wait(null, false, 29) does not match expected value. Failed asserting that 29 matches expected 30. /home/travis/build/eMAGTechLabs/RabbitMqBundle/RabbitMq/Consumer.php:95 /home/travis/build/eMAGTechLabs/RabbitMqBundle/Tests/RabbitMq/ConsumerTest.php:398

andreea-anamaria · 2020-10-15T09:35:53Z

Tests/RabbitMq/ConsumerTest.php

+        $consumer->consume(1);
+    }
+
+    public function testTimeoutWaitWontWaitPastGracefulMaxExecutionTimeout()


Please rerun this test and check the failing reason:
2) OldSound\RabbitMqBundle\Tests\RabbitMq\ConsumerTest::testTimeoutWaitWontWaitPastGracefulMaxExecutionTimeout Expectation failed for method name is "wait" when invoked 1 time(s) Parameter 2 for invocation PhpAmqpLib\Channel\AbstractChannel::wait(null, false, 9) does not match expected value. Failed asserting that 9 matches expected 10. /home/travis/build/eMAGTechLabs/RabbitMqBundle/RabbitMq/Consumer.php:95 /home/travis/build/eMAGTechLabs/RabbitMqBundle/Tests/RabbitMq/ConsumerTest.php:435

Check also:
4) OldSound\RabbitMqBundle\Tests\RabbitMq\DynamicConsumerTest::testTimeoutWaitWontWaitPastGracefulMaxExecutionTimeout Expectation failed for method name is "wait" when invoked 1 time(s) Parameter 2 for invocation PhpAmqpLib\Channel\AbstractChannel::wait(null, false, 9) does not match expected value. Failed asserting that 9 matches expected 10. /home/travis/build/eMAGTechLabs/RabbitMqBundle/RabbitMq/Consumer.php:95 /home/travis/build/eMAGTechLabs/RabbitMqBundle/Tests/RabbitMq/ConsumerTest.php:435

andreea-anamaria · 2020-10-15T13:29:21Z

Tests/RabbitMq/ConsumerTest.php

 use PHPUnit\Framework\TestCase;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface as ContractsEventDispatcherInterface;
+use Symfony\Bridge\PhpUnit\ClockMock;


Please check the possible issues introduced: https://scrutinizer-ci.com/g/eMAGTechLabs/RabbitMqBundle/inspections/29e62c38-e196-4a29-a242-e22f92f6740c/issues/files/Tests/RabbitMq/ConsumerTest.php?status=new&orderField=path&order=asc&honorSelectedPaths=0

…estTimeoutWait' tests

constantable · 2020-10-16T16:19:03Z

I'm sorry for the multiple unintended code changes. I was not expecting further code changes would be reflected on the PR once it has been submitted.
Fixed issues above, please review this again.
Thanks!

andreea-anamaria · 2020-10-29T09:08:09Z

RabbitMq/Consumer.php

-     * Choose the timeout to use for the $this->getChannel()->wait() method.
+     * @return int
+     */
+    public function getTimeoutWait()


Please return type here and not in comment, as:
public function getTimeoutWait(): int

andreea-anamaria · 2020-10-29T09:08:19Z

RabbitMq/Consumer.php

+    /**
+     * @param int $timeoutWait
+     */
+    public function setTimeoutWait($timeoutWait)


Please add return type and parameter type here and not in comment.

andreea-anamaria · 2020-10-29T09:08:43Z

RabbitMq/Consumer.php

-     *  }
+     * @return int
     */
    private function chooseWaitTimeout()


Please return type here and not in comment.

andreea-anamaria · 2020-10-29T09:11:03Z

RabbitMq/Consumer.php

-            'timeoutType' => self::TIMEOUT_TYPE_IDLE,
-            'seconds' => $this->getIdleTimeout(),
-        );
+    /**


Please remove the comment.

Do you mean I should completely remove @param annotations?

Yes. Once you've added typehint for parameter, comment it's useless

andreea-anamaria · 2020-10-29T09:11:32Z

RabbitMq/Consumer.php

+    /**
+     * @return \DateTime|null
+     */
+    protected function getLastActivityDateTime()


Please return type here and not in comment.

andreea-anamaria · 2020-10-29T10:11:21Z

RabbitMq/Consumer.php

    }

    /**
     * @param int $timeoutWait


Please also remove comment

andreea-anamaria · 2020-10-29T10:11:38Z

RabbitMq/Consumer.php

    }

    /**
     * @return int


Please also remove comment.

andreea-anamaria · 2020-10-29T10:11:46Z

RabbitMq/Consumer.php

    /**
     * Choose the timeout wait (in seconds) to use for the $this->getChannel()->wait() method.
     *
     * @return int


Please also remove comment.

andreea-anamaria · 2020-10-29T10:11:55Z

RabbitMq/Consumer.php


    /**
     * @return \DateTime|null
     */


Please also remove comment.

andreea-anamaria · 2020-10-29T10:25:52Z

Please checkout failing tests: https://scrutinizer-ci.com/g/eMAGTechLabs/RabbitMqBundle/inspections/4c087a48-4115-441e-abb3-40eb74100361

andreea-anamaria · 2020-10-29T10:51:00Z

RabbitMq/Consumer.php

-                'timeoutType' => self::TIMEOUT_TYPE_GRACEFUL_MAX_EXECUTION,
-                'seconds' => $allowedExecutionSeconds,
-            );
+        if ($this->getTimeoutWait()) {


Please check if (!is_null($this->getTimeoutWait())) or if ($this->getTimeoutWait() !== null) because it returns an integer value. Check also: https://scrutinizer-ci.com/g/eMAGTechLabs/RabbitMqBundle/inspections/d889e465-34c7-4f57-bea6-6300aa404c64/issues/files/RabbitMq/Consumer.php?status=new&orderField=path&order=asc&honorSelectedPaths=0

Consumer timeout wait

d8fcdc4

constantable requested a review from mihaileu as a code owner October 14, 2020 13:34

andreea-anamaria reviewed Oct 15, 2020

View reviewed changes

constantable force-pushed the master branch from c2bf2cd to d8fcdc4 Compare October 16, 2020 14:27

Fixed 'testTimeoutWaitWontWaitPastGracefulMaxExecutionTimeout' and 't…

120e0e4

…estTimeoutWait' tests

mihaileu requested review from draage2572, stefanovik and theodormanolescu October 19, 2020 08:14

andreea-anamaria reviewed Oct 29, 2020

View reviewed changes

Function return types

ba5f254

andreea-anamaria reviewed Oct 29, 2020

View reviewed changes

Comments removed

c0ac55a

getTimeoutWait return type fixed

6313cd9

andreea-anamaria reviewed Oct 29, 2020

View reviewed changes

Check getTimeoutWait() is integer gt 0

82a3feb

andreea-anamaria approved these changes Oct 29, 2020

View reviewed changes

mihaileu approved these changes Nov 18, 2020

View reviewed changes

mihaileu merged commit 5542098 into eMAGTechLabs:master Nov 18, 2020

Consumer timeout wait #36

Consumer timeout wait #36

Uh oh!

Conversation

constantable commented Oct 14, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

constantable commented Oct 16, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

constantable Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreea-anamaria commented Oct 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

constantable Oct 29, 2020 •

edited

Loading