-
Notifications
You must be signed in to change notification settings - Fork 434
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DBAL Transport #54
DBAL Transport #54
Conversation
pkg/dbal/DbalConnectionFactory.php
Outdated
*/ | ||
private function establishConnection() | ||
{ | ||
return $this->registry->getConnection($this->config['connectionName']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we should call connect at this stage
pkg/dbal/DbalConsumer.php
Outdated
} catch (\LogicException $e) { | ||
$this->dbal->rollBack(); | ||
throw $e; | ||
} catch (\Exception $e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need two catch statements here?
pkg/dbal/DbalConsumer.php
Outdated
$message->setId($dbalMessage['id']); | ||
$message->setBody($dbalMessage['body']); | ||
$message->setPriority((int) $dbalMessage['priority']); | ||
$message->setRedelivered((bool) $dbalMessage['redelivered']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no such thing for now
{ | ||
$builder | ||
->children() | ||
->scalarNode('connectionName') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other transport factories use snake_case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
partly address #32 |
pkg/enqueue-bundle/Tests/Unit/EnqueueBundleTest.php
$clientMessage->setMessageId($message->getMessageId()); | ||
$clientMessage->setTimestamp($message->getTimestamp()); | ||
$clientMessage->setPriority(MessagePriority::NORMAL); | ||
$clientMessage->setDelay($message->getDelay()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setReplyTo
setCorreltationId
$transportMessage->setProperties($properties); | ||
$transportMessage->setMessageId($message->getMessageId()); | ||
$transportMessage->setTimestamp($message->getTimestamp()); | ||
$transportMessage->setDelay($message->getDelay()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replyTo,correlationId
pkg/dbal/Client/DbalDriver.php
Outdated
throw new \LogicException('Topic name parameter is required but is not set'); | ||
} | ||
|
||
$topic = $this->createRouterTopic(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to use
$queue = $this->createQueue($this->config->getRouterQueueName());
* | ||
* @return DbalMessage|null | ||
*/ | ||
public function receive($timeout = 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
timeout in miliseconds, if you need seconds do $timeout / 1000
pkg/dbal/LICENSE
Outdated
@@ -0,0 +1,20 @@ | |||
The MIT License (MIT) | |||
Copyright (c) 2016 Kotliar Maksym |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oro copyright
{ | ||
new DbalDriver( | ||
$this->createPsrContextMock(), | ||
new Config('', '', '', '', '', '') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Config::create()
pkg/dbal/composer.json
Outdated
"name": "enqueue/dbal", | ||
"type": "library", | ||
"description": "Message Queue Doctrine DBAL Transport", | ||
"keywords": ["messaging", "queue", "dbal"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doctrine
LGTM, you can start with SQS. I'll take care of the rest. |
No description provided.