Merge lp://staging/~sinzui/juju-ci-tools/azure-arm-substrate into lp://staging/juju-ci-tools

Proposed by Curtis Hovey
Status: Merged
Merged at revision: 1415
Proposed branch: lp://staging/~sinzui/juju-ci-tools/azure-arm-substrate
Merge into: lp://staging/juju-ci-tools
Diff against target: 409 lines (+226/-44)
2 files modified
tests/test_winazurearm.py (+127/-36)
winazurearm.py (+99/-8)
To merge this branch: bzr merge lp://staging/~sinzui/juju-ci-tools/azure-arm-substrate
Reviewer Review Type Date Requested Status
Martin Packman (community) Approve
Review via email: mp+295132@code.staging.launchpad.net

Description of the change

Add support to delete a single instance in Azure ARM.

This branch adds delete_instance() and the helper find_vm_instance() to the winazurearm.py lib.
I The branch is long so I am not integrating this into substrate.py just yet.

This branch contains a few drive by fixes and a refactoring of the tests.

There are cases where the poller can be None when deleting, but after I merged my branch, I saw it was
often None. Oops.
1. Deleting a resource group did not return the poller.
   This was broken just before I merged my first branch.
2. A call to log.debug() didn't pass the name of the group that had the None poller.

I didn't like mocking the call to ResourceGroupDetails.delete. Since the client is mocked,
I decided to mock the client delete() methods so that I could check that the call args matched
the documentation. i like this refactoring, but it changed most of the tests in the module.

There is nothing wrong with delete, but since the azure lib is incomplete.
I want to clearly know what we think the call args are in case it changes.

My next branch will update substrate.py to call delete_instance().

To post a comment you must log in.
Revision history for this message
Martin Packman (gz) wrote :

Looks good! Some minor comments inline.

review: Approve
1413. By Curtis Hovey

Updates per review.

1414. By Curtis Hovey

Updated docstring.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches