Skip to content

[wip] Azure support#645

Closed
sylfabre wants to merge 5 commits into
php-enqueue:masterfrom
sylfabre:azure_queue_storage
Closed

[wip] Azure support#645
sylfabre wants to merge 5 commits into
php-enqueue:masterfrom
sylfabre:azure_queue_storage

Conversation

@sylfabre

Copy link
Copy Markdown
Contributor

No description provided.

@makasim

makasim commented Nov 20, 2018

Copy link
Copy Markdown
Member

@sylfabre there are some conflicts. could you fix them, please?

@sylfabre

Copy link
Copy Markdown
Contributor Author

@makasim my colleague @PaulRouss3l is working on it now. He'll do it tomorrow.

@makasim

makasim commented Nov 21, 2018

Copy link
Copy Markdown
Member

@makasim

makasim commented Nov 21, 2018

Copy link
Copy Markdown
Member

It needs more tests, at least spec tests should be implemented

{
InvalidMessageException::assertMessageInstanceOf($message, AzureStorageMessage::class);

if(false === $requeue) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it should publish a message on requeue == true

*/
public function setDeliveryDelay(int $deliveryDelay = null): Producer
{
throw new DeliveryDelayNotSupportedException();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do not throw exception if null is passed

*/
public function setPriority(int $priority = null): Producer
{
throw new PriorityNotSupportedException();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do not throw exception if null is passed

@makasim

makasim commented Nov 21, 2018

Copy link
Copy Markdown
Member

Code styles should be fixed, you could run php-cs-fixer to auto-fix them.

@makasim

makasim commented Nov 21, 2018

Copy link
Copy Markdown
Member

root phpunit.xml.dist file should be updated too, so tests are executed by travis.

@PaulRouss3l

Copy link
Copy Markdown

I'm currently implementing the tests, and will update with the comment you made above =)

@PaulRouss3l

Copy link
Copy Markdown

still wip

if (true === $requeue) {
$factory = new AzureStorageConnectionFactory($this->client);
$context = $factory->getContext();
$context = new AzureStorageContext($this->client);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think context should be passed in consumer constructor. The context should have a getClient public method

@makasim

makasim commented Nov 22, 2018

Copy link
Copy Markdown
Member

That was decided to release it as a stand-alone package https://github.com/assoconnect/enqueue-azure

Closing PR. Thank you guys for your effort!

@makasim makasim closed this Nov 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants