Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Feature/sqs bulk messages#6

Merged
tejerka merged 7 commits intosemji:mastersemji/queue-client:masterfrom
neverdane:feature/sqs-bulk-messagesneverdane/queue-client:feature/sqs-bulk-messagesCopy head branch name to clipboard
Jan 27, 2016
Merged

Feature/sqs bulk messages#6
tejerka merged 7 commits intosemji:mastersemji/queue-client:masterfrom
neverdane:feature/sqs-bulk-messagesneverdane/queue-client:feature/sqs-bulk-messagesCopy head branch name to clipboard

Conversation

@neverdane
Copy link
Contributor

We used to iterate through each message and therefore send each message individually which added a performance issue. Instead, we prefer to send pools of 10 messages in order to minimize API calls.

The addMessage method tested if the queues to process
were contained in an array in order to iterate through them
otherwise we assumed it was the queueName only. This
condition was useless and duplicated some lines of code as
we could directly cast the given queues as an array.
The addMessages method called the addMessage for each message.
This addMessage resolved each time the alias queue name whereas
it was already done the first time. We only had to resolve it at the
beginning of addMessages and call the adapter's method for each
message.
We have common behaviours in some adapters methods.
In order to share these behaviours, we inherit all Adapters
from an AbstractAdapter (which is empty for the moment).
Instead of iterating through each message, we can call the
method addMessages of the adapter which can now be
overridden by each adapter as it inherits from AbstractAdapter.
We used to iterate through each message and therefore send
each message individually which added a performance issue.
Instead, we prefer to send pools of 10 messages in order to
minimize API calls.
Some addMessages conditions weren't tested which reduced
the unit tests code coverage.
@neverdane neverdane force-pushed the feature/sqs-bulk-messages branch from b04c824 to a9914db Compare January 26, 2016 22:10
tejerka added a commit that referenced this pull request Jan 27, 2016
@tejerka tejerka merged commit f35e8e1 into semji:master Jan 27, 2016
Copy link
Contributor

Choose a reason for hiding this comment

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

$this
    ->given($NullAdapter = new \ReputationVIP\QueueClient\Adapter\NullAdapter())
    ->then
        ->object($NullAdapter->addMessages('testQueue', ['test Message one']))
            ->isInstanceOf('\ReputationVIP\QueueClient\Adapter\AdapterInterface')
;

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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.