squigg / azure-queue-laravel

PHP Laravel Queue Driver package for Microsoft Azure Storage Queues
43 stars 24 forks source link

QueueEndpoint argument support #21

Closed dragomirt closed 2 years ago

dragomirt commented 2 years ago

Hey! Not really an issue here, but more of a feature question.

On my current project I'm working with a local queue which runs through Azurite. To connect to the instance it uses a link of the following format: http://127.0.0.1:10001/<storage_name>. Therefore it's not compatible with the package as is, since there is only the "Endpoint Suffix" option available, which does not work with this URL format.

It's not really documented from what I saw, but apparently there is an option called "QueueEndpoint", which can take in a custom URL, and does support the format above. I did test it with the aforementioned project, and it does indeed work just as expected.

So my question is, would it be useful if I submit a PR with a new config argument that has that option available?

References:

This argument in the query string in the official package: here

This argument used by the storage emulator: here

And if you do agree that this addition would be useful, how would you like the config to be named? The endpoint setting is already occupied therefore I'm thinking of "endpoint_url" or "endpoint_custom" of sorts.

Thank you for reading this and really curious to know your opinion on this addition ;)

Will try to make a PR soon so to have something to click through.

squigg commented 2 years ago

Hey, thanks for taking the time. It sounds like a very sensible proposal.

Shame I already used endpoint a little poorly in existing config. I think either endpoint_url or possibly queue_endpoint to align with the connection string key.

Perhaps for clarity we could deprecate endpoint and implement backward compatible additional config option as endpoint_sufffix so all the config params are lined up.

Will happily review a PR for this change. Let me know if you have any further questions :)

dragomirt commented 2 years ago

I think having it as a queue_endpoint would be the most straightforward, and it doesn't interfere with the naming logic, since there are the QUEUE_ENDPOINT and QUEUE_ENDPOINTSUFFIX env keys, which are distinctive enough I assume :D .

squigg commented 2 years ago

Thanks, merged and released as 8.2.0.

When I get a moment I'll backport it to 7.x also, just need to figure out where my branches are at.