implementation for #74

[xbranko]

This is my first attempt at contributing back via git. Not sure how it all works, but hopefully this will be of help. It worked in my environment. Don't know how to exclude a change (to pom, we hare don't have commons-logging 1.1, but 1.1.1).

[carterpage]

Good idea. A couple of things:

1. Don't change existing interface implementations, extend them and add new methods.
2. Don't change existing tests. Add new ones.

Some day I'd like to extend this to other cloud providers, at which point it will make sense to refactor credentials entirely, but I'm not ready to do that today. I can't break backward compatibility. Please make the changes requested and resubmit.

[carterpage]

ping

[xbranko]

Sorry I got busy with my project once I got it unblocked. I will do as you suggest, and offer it via another pull request.

[xbranko]

I tried to do as you asked, but have run into a problem. AmazonAwsSQSConnector that I wanted to extend defines _amazonSQS and _amazonSNS as private final fields that are initialized in its constructor. AWSCredentials object is being passed to their constructors, and at that point the derived class can't offer different implementation, because it is too late.

Did my initial solution break any of your tests or apps? If it didn't, would you reconsider accepting it?
If you know of a better (more elegant, cleaner) way to achieve this, please let me know, or if it is easier feel free to do it.

[carterpage]

This library is used by lots of developers, so we can't simply change public method signatures. You'd be surprised, but someone is almost certainly using those directly. This change will break their build. Feel free to change the private final fields to non-final, and if needed, protected. Let's see if there's another way to do this.

[xbranko]

One way to do it would be to have the SQSConnectorFactory define new getInstance method(s) that take a newly defined credentials object (e.g. of class NevadoCredentials). That way whether there are two, three or more parameters needed, the signature would not need to change. Yet whoever used the current interface would not be adversely affected. Would that work?

[carterpage]

I think that would work. I want to have a Credentials object at some point
besides. It may take a couple of iterations to get the signatures correct,
but I think that's a good direction.
On Feb 5, 2014 2:06 PM, "xbranko" notifications@github.com wrote:

One way to do it would be to have the SQSConnectorFactory define new
getInstance method(s) that take a newly defined credentials object (e.g. of
class NevadoCredentials). That way whether there are two, three or more
parameters needed, the signature would not need to change. Yet whoever used
the current interface would not be adversely affected. Would that work?

Reply to this email directly or view it on GitHubhttps://github.com//pull/75#issuecomment-34225103
.

[twicksell]

@carterpage can we get an update on this issue and PR? I also have a need for session credentials and I'd really like to continue using Nevado.

[carterpage]

Some work is still required. The pull request as it currently stands is inadmissible because it breaks backwards compatibility. @xbranko suggested an approach, which is sounds great, but it hasn't been submitted. If you would like to take a stab at it, please do. You can deprecate the old methods, but not remove them. I unfortunately don't have the time to implement it any time soon. Thanks for understanding.