Skip to content

Conversation

ATGE
Copy link
Contributor

@ATGE ATGE commented Mar 20, 2020

Ability to set which DC an image has a copy in.

@ATGE ATGE requested a review from allmightyspiff March 20, 2020 20:02
@coveralls
Copy link

coveralls commented Mar 20, 2020

Coverage Status

Coverage increased (+0.6%) to 94.468% when pulling 212472f on ATGE:issue1234 into dc173c1 on softlayer:master.

Copy link
Member

@allmightyspiff allmightyspiff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor style changes, otherwise looks good. Thanks.

output_error.format(location_name, image_id)
)

locations_ids.append(matching_location.get('id'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd change this to locations_ids.append(matching_location) so that you can just pass that list directly to the API and you can remove the locations_ids = [{'id': location_id} for location_id in locations] lines in your other two functions.

:param int image_id: The ID of the image.
:param list location_names: A list of location names strings.
:returns: A list of locations IDs associated with the given location
keynames in the image id.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this 1 line

break
if matching_location.get('id') is None:
raise exceptions.SoftLayerError(
output_error.format(location_name, image_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this 1 line

@click.command()
@click.argument('identifier')
@click.option('--add/--remove',
default=False,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default to True, so the default behavior will be to add dcs, and put this on the same line as --add/--remove, help can be on its own line though.

@allmightyspiff allmightyspiff linked an issue Mar 20, 2020 that may be closed by this pull request
@allmightyspiff allmightyspiff merged commit 4bb26eb into softlayer:master Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

image edit support changing datacenters
3 participants