Skip to content

added queue prefix to match the framework #3576

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

Merged
merged 1 commit into from
Dec 1, 2015
Merged

added queue prefix to match the framework #3576

merged 1 commit into from
Dec 1, 2015

Conversation

pulkitjalan
Copy link
Contributor

Updated queue config file to match sqs prefix changes in the framework

taylorotwell added a commit that referenced this pull request Dec 1, 2015
added queue prefix to match the framework
@taylorotwell taylorotwell merged commit 7f216a4 into laravel:master Dec 1, 2015
@taylorotwell
Copy link
Member

Can you send this to develop branch instead?

@fideloper
Copy link
Contributor

That URL also assumes us-east-1, which is probably OK, but not necessarily obvious to users new to aws (maybe use a placeholder one there too?).


Other random note: SQS provides a way to get the URL of the queue by queue name (http://docs.aws.amazon.com/aws-sdk-php/v3/api/api-sqs-2012-11-05.html#getqueueurl), which I mention just in case that's a preferred way to drop the need for the 'prefix'.

Then perhaps the code could detect if a url is used (preg_match("/(https?\:\/\/)/", $input_line, $output_array);) and decide to make that call if not.

(although since that makes extra api calls, hard-coding that is probably "better", so ignore that dump of bile, since it's just making extra work).

@pulkitjalan
Copy link
Contributor Author

@fideloper I mostly copied the iron config, it looks like that also has the same us-east-1 region in the url. Yea the extra api call was the reason the function is not used.

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