Merge lp://staging/~abentley/juju-release-tools/azure-images-2 into lp://staging/juju-release-tools

Proposed by Aaron Bentley
Status: Merged
Merged at revision: 323
Proposed branch: lp://staging/~abentley/juju-release-tools/azure-images-2
Merge into: lp://staging/juju-release-tools
Prerequisite: lp://staging/~abentley/juju-release-tools/azure-images
Diff against target: 349 lines (+129/-64)
3 files modified
azure_image_streams.py (+1/-1)
make_image_streams.py (+38/-16)
tests/test_make_image_streams.py (+90/-47)
To merge this branch: bzr merge lp://staging/~abentley/juju-release-tools/azure-images-2
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+300133@code.staging.launchpad.net

Commit message

Add azure support to make_aws_image_streams.

Description of the change

This branch adds Azure support to make_aws_image_streams.

It also renames the script to make_image_streams.

It is based on lp:~abentley/juju-release-tools/azure-images, and applies the functionality to actually generating simplestreams.

With these changes, supplying --azure will cause the script to additionally generate Azure data.

Supporting this meant some reshuffling of functionality, eventually producing make_aws_items (equivalent to make_azure_items) and write_item_streams (instead of write_streams).

mocked_iter_region was extracted from iter_centos_images so that it could be reused in TestMakeAwsItems.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

I have a question inline.

review: Needs Information (code)
Revision history for this message
Aaron Bentley (abentley) wrote :

On 2016-07-18 01:12 PM, Curtis Hovey wrote:
>> + parser.add_argument('--azure', help='Generate Azure streams also.',
>
> This seems odd. What if I only want azure? Could this be a choice of aws, azure, ALL?

My rationale was that AWS is production-ready and Azure support is still
experimental. Should we want to re-generate images before Azure support
is production-ready, we would leave the flag off.

I was also minimizing the command-line changes (i.e. just the name).

However, I'm happy to change this. While testing, it is more convenient
to just generate Azure streams. Maybe something like:

make_image_streams STREAMS [--cloud [aws|azure|all]]

And the default would be "all"?

Revision history for this message
Curtis Hovey (sinzui) wrote :

I was thinking "all" as make streams for all clouds (aws and azure today).

Revision history for this message
Aaron Bentley (abentley) wrote :

So you want it to be explicit, not the default?

Revision history for this message
Curtis Hovey (sinzui) wrote :

I do prefer explicit.

350. By Aaron Bentley

Merged azure-images into azure-images-2.

351. By Aaron Bentley

Use explicit cloud parameter.

Revision history for this message
Aaron Bentley (abentley) wrote :

Updated to use an explicit cloud parameter, per your request.

Revision history for this message
Curtis Hovey (sinzui) wrote :

Thank you.

review: Approve (code)

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