Conversation
|
By analyzing the blame information on this pull request, we identified @ryansydnor, @rutsky and @kouk to be potential reviewers |
|
cool! |
| identifier='publickey', | ||
| secret_identifier='privatekey', | ||
| keypair_name="test_key", | ||
| keypair_name="latent_buildbot_slave", |
There was a problem hiding this comment.
Please use latent_buildbot_worker instead of latent_buildbot_slave.
There was a problem hiding this comment.
The test fixture sets up latent_buildbot_slave, I can change the test fixture to use latent_buildbot_worker and update the whole test suite , but I would also have to change the defaults in ec2.py#L76
Want me to change it for both, or is there some backwards compatibility that needs to be maintained? for _slave?
There was a problem hiding this comment.
Default can't be changed due to backward compatibility.
Current use of default value is deprecated and warning is emitted, if user doesn't explicitly specify value for keypair_name or security_name.
Please change value that is used in test.
There was a problem hiding this comment.
I changed the value from slave to worker in every test that wasn't checking for deprecation warnings.. there's quite a few that did that.
|
Lgtm! (Other than rutsky's comment) |
Current coverage is 84.23%@@ master #2164 diff @@
==========================================
Files 333 333
Lines 32001 32001
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 26920 26955 +35
+ Misses 5081 5046 -35
Partials 0 0
|
|
Let me know if any other changes are required. |
|
LGTM |
|
Once this is merged, I'll submit the boto2 -> boto3 upgrade as a new PR. |
7 participants
Increase test coverage in preparation for boto2 -> boto3 upgrade