Skip to content
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

Remove Aws.config #293

Closed
3 tasks done
phstc opened this issue Dec 20, 2016 · 5 comments
Closed
3 tasks done

Remove Aws.config #293

phstc opened this issue Dec 20, 2016 · 5 comments
Milestone

Comments

@phstc
Copy link
Collaborator

phstc commented Dec 20, 2016

We should drop any AWS credentials setup from Shoryuken, like this:

Shoryuken.configure_client do |config|
  config.aws = {

In favor of leveraging Aws.config.

aws-sdk is very powerful and supports a few different ways to configure it, we shouldn't duplicate this logic in Shoryuken. This would also "resolve" the problem about people complaining that they've configured the creds in shoryuken.yml, but it isn't picked up in client mode (Rails), they would know that they need to configure that by themselves.

  • Drop https://github.com/phstc/shoryuken/blob/master/lib/shoryuken/aws_config.rb
  • Update README and Wiki references related to the Aws credentials/config setup. Explaining how to use the Aws.config
    • Add a link to Aws.config for more details
    • Explain that it should be configured using an initializer or ENV variables or IAM roles or any other way described in aws-sdk that's more convenient for them. As long as it's properly configured, Shoryuken will use it.
  • Remove SNS references in the code. Shoryuken does not use SNS, people must refer to aws-sdk instead

The maximum we can provide is a setter sqs= to allow people to set the SQS Client, in case the have a specific need, such as sqs_endpoint or they want to have different credentials other than the ones in Aws.config. I'm not totally convinced about this setter yet, but that is the maximum we can provide.

We currently support receive_message in the shoryuken.yml, but I think people only use it to for attributes, and because of the FIFO queue, we ask for all by default.

@phstc
Copy link
Collaborator Author

phstc commented Dec 20, 2016

@mariokostelac WDYT? This should be part of the next version 3.0.0, all together with #291.

If ok with that and if you have a spare time, could you work on that?

@phstc phstc modified the milestone: 3.0.0 Dec 20, 2016
@mariokostelac
Copy link
Contributor

@phstc give me few days to take a look, probably end of the week.

@mbriggs
Copy link

mbriggs commented Feb 13, 2017

I have most of my AWS resources in a different region from my SQS queues, since amazon only supports FIFO in a few regions. If you go this way, please provide a way to override region ONLY for shoryuken :)

@phstc
Copy link
Collaborator Author

phstc commented Feb 14, 2017

@mbriggs would Shoryuken.sqs= ... expecting a Aws::SQS::Client.new work for you?

@mbriggs
Copy link

mbriggs commented Feb 14, 2017

yeah, totally :) just need some kind of way to configure stuff outside of the global sdk configuration

@phstc phstc mentioned this issue Feb 17, 2017
phstc added a commit that referenced this issue Feb 17, 2017
@phstc phstc closed this as completed Feb 17, 2017
@phstc phstc added the v3 label Feb 19, 2017
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

No branches or pull requests

3 participants