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

Add profile_name support to SQS #2459

Merged
merged 7 commits into from Aug 25, 2014

Conversation

Projects
None yet
2 participants
@chrishenry
Contributor

chrishenry commented Jul 28, 2014

No description provided.

@danielgtaylor danielgtaylor added FPS and removed FPS labels Jul 29, 2014

@danielgtaylor

This comment has been minimized.

Show comment
Hide comment
@danielgtaylor

danielgtaylor Jul 29, 2014

Member

Thanks for the pull request! This looks good, but is missing tests to cover this new functionality. You can see examples in the tests/unit/sqs directory.

Member

danielgtaylor commented Jul 29, 2014

Thanks for the pull request! This looks good, but is missing tests to cover this new functionality. You can see examples in the tests/unit/sqs directory.

@chrishenry

This comment has been minimized.

Show comment
Hide comment
@chrishenry

chrishenry Jul 29, 2014

Contributor

You're welcome! Before submitting, I was looking for tests against other AWS services that covered profile_name and couldn't find any, but maybe I wasn't looking for the right thing.

Could you point me towards an example of a unit test for another service that covers profile_name

Contributor

chrishenry commented Jul 29, 2014

You're welcome! Before submitting, I was looking for tests against other AWS services that covered profile_name and couldn't find any, but maybe I wasn't looking for the right thing.

Could you point me towards an example of a unit test for another service that covers profile_name

@danielgtaylor

This comment has been minimized.

Show comment
Hide comment
@danielgtaylor

danielgtaylor Jul 29, 2014

Member

Sorry, you should be able to mock AWSQueryConnection's constructor and then ensure that profile_name gets passed through to it.

You can also try to create a new connection from the list returned by regions() to ensure that works as expected.

Member

danielgtaylor commented Jul 29, 2014

Sorry, you should be able to mock AWSQueryConnection's constructor and then ensure that profile_name gets passed through to it.

You can also try to create a new connection from the list returned by regions() to ensure that works as expected.

@chrishenry

This comment has been minimized.

Show comment
Hide comment
@chrishenry

chrishenry Aug 12, 2014

Contributor

@danielgtaylor Hey Dan, I was able to write a test with a mock that worked in Python2 here, but not Python3, so I removed it.

So as of now, that test doesn't confirm a whole lot. Any advice on what I can assert / mock to make this a stronger test?

Contributor

chrishenry commented Aug 12, 2014

@danielgtaylor Hey Dan, I was able to write a test with a mock that worked in Python2 here, but not Python3, so I removed it.

So as of now, that test doesn't confirm a whole lot. Any advice on what I can assert / mock to make this a stronger test?

@danielgtaylor

This comment has been minimized.

Show comment
Hide comment
@danielgtaylor

danielgtaylor Aug 13, 2014

Member

It looks like you can just assert that the self.service_connection.profile_name exists and is set to 'prod'. Once that's in I think this is good to merge.

Member

danielgtaylor commented Aug 13, 2014

It looks like you can just assert that the self.service_connection.profile_name exists and is set to 'prod'. Once that's in I think this is good to merge.

@chrishenry

This comment has been minimized.

Show comment
Hide comment
@chrishenry

chrishenry Aug 23, 2014

Contributor

@danielgtaylor Hey, have you had a moment to take a look at this?

Contributor

chrishenry commented Aug 23, 2014

@danielgtaylor Hey, have you had a moment to take a look at this?

@danielgtaylor

This comment has been minimized.

Show comment
Hide comment
@danielgtaylor

danielgtaylor Aug 25, 2014

Member

@chrishenry sorry about that! This looks great, thanks! 👍

Member

danielgtaylor commented Aug 25, 2014

@chrishenry sorry about that! This looks great, thanks! 👍

danielgtaylor added a commit that referenced this pull request Aug 25, 2014

Merge pull request #2459 from behanceops/develop
Add profile_name support to SQS. Fixes #2459.

@danielgtaylor danielgtaylor merged commit 3837951 into boto:develop Aug 25, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment