Skip to content

Commit 4776e0a

Browse files
stephenfinstephenfin
committed
image: Make better use of argparse
Simplify some logic by using a common 'dest' for mutually exclusive options. Change-Id: Ie5f3600672953f40be52de51e84717c8912ddaf8 Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
1 parent 2290b38 commit 4776e0a

2 files changed

Lines changed: 116 additions & 149 deletions

File tree

‎openstackclient/image/v2/image.py‎

Lines changed: 80 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -325,32 +325,44 @@ def get_parser(self, prog_name):
325325
protected_group.add_argument(
326326
"--protected",
327327
action="store_true",
328+
dest="is_protected",
329+
default=None,
328330
help=_("Prevent image from being deleted"),
329331
)
330332
protected_group.add_argument(
331333
"--unprotected",
332-
action="store_true",
334+
action="store_false",
335+
dest="is_protected",
336+
default=None,
333337
help=_("Allow image to be deleted (default)"),
334338
)
335339
public_group = parser.add_mutually_exclusive_group()
336340
public_group.add_argument(
337341
"--public",
338-
action="store_true",
342+
action="store_const",
343+
const="public",
344+
dest="visibility",
339345
help=_("Image is accessible to the public"),
340346
)
341347
public_group.add_argument(
342348
"--private",
343-
action="store_true",
349+
action="store_const",
350+
const="private",
351+
dest="visibility",
344352
help=_("Image is inaccessible to the public (default)"),
345353
)
346354
public_group.add_argument(
347355
"--community",
348-
action="store_true",
356+
action="store_const",
357+
const="community",
358+
dest="visibility",
349359
help=_("Image is accessible to the community"),
350360
)
351361
public_group.add_argument(
352362
"--shared",
353-
action="store_true",
363+
action="store_const",
364+
const="shared",
365+
dest="visibility",
354366
help=_("Image can be shared"),
355367
)
356368
parser.add_argument(
@@ -440,18 +452,12 @@ def take_action(self, parsed_args):
440452
# a single value for the pair of options because the default must be
441453
# to do nothing when no options are present as opposed to always
442454
# setting a default.
443-
if parsed_args.protected:
444-
kwargs['is_protected'] = True
445-
if parsed_args.unprotected:
446-
kwargs['is_protected'] = False
447-
if parsed_args.public:
448-
kwargs['visibility'] = 'public'
449-
if parsed_args.private:
450-
kwargs['visibility'] = 'private'
451-
if parsed_args.community:
452-
kwargs['visibility'] = 'community'
453-
if parsed_args.shared:
454-
kwargs['visibility'] = 'shared'
455+
if parsed_args.is_protected is not None:
456+
kwargs['is_protected'] = parsed_args.is_protected
457+
458+
if parsed_args.visibility is not None:
459+
kwargs['visibility'] = parsed_args.visibility
460+
455461
if parsed_args.project:
456462
kwargs['owner_id'] = common.find_project(
457463
identity_client,
@@ -557,16 +563,20 @@ def take_action(self, parsed_args):
557563
if volume_client.api_version >= api_versions.APIVersion('3.1'):
558564
mv_kwargs.update(
559565
visibility=kwargs.get('visibility', 'private'),
560-
protected=bool(parsed_args.protected),
566+
protected=bool(parsed_args.is_protected),
561567
)
562568
else:
563-
if kwargs.get('visibility') or parsed_args.protected:
569+
if (
570+
parsed_args.visibility or
571+
parsed_args.is_protected is not None
572+
):
564573
msg = _(
565574
'--os-volume-api-version 3.1 or greater is required '
566575
'to support the --public, --private, --community, '
567576
'--shared or --protected option.'
568577
)
569578
raise exceptions.CommandError(msg)
579+
570580
response, body = volume_client.volumes.upload_to_image(
571581
source_volume.id,
572582
parsed_args.force,
@@ -637,37 +647,37 @@ def get_parser(self, prog_name):
637647
public_group = parser.add_mutually_exclusive_group()
638648
public_group.add_argument(
639649
"--public",
640-
dest="public",
641-
action="store_true",
642-
default=False,
650+
action="store_const",
651+
const="public",
652+
dest="visibility",
643653
help=_("List only public images"),
644654
)
645655
public_group.add_argument(
646656
"--private",
647-
dest="private",
648-
action="store_true",
649-
default=False,
657+
action="store_const",
658+
const="private",
659+
dest="visibility",
650660
help=_("List only private images"),
651661
)
652662
public_group.add_argument(
653663
"--community",
654-
dest="community",
655-
action="store_true",
656-
default=False,
664+
action="store_const",
665+
const="community",
666+
dest="visibility",
657667
help=_("List only community images"),
658668
)
659669
public_group.add_argument(
660670
"--shared",
661-
dest="shared",
662-
action="store_true",
663-
default=False,
671+
action="store_const",
672+
const="shared",
673+
dest="visibility",
664674
help=_("List only shared images"),
665675
)
666676
public_group.add_argument(
667677
"--all",
668-
dest="all",
669-
action="store_true",
670-
default=False,
678+
action="store_const",
679+
const="all",
680+
dest="visibility",
671681
help=_("List all images"),
672682
)
673683
parser.add_argument(
@@ -724,6 +734,7 @@ def get_parser(self, prog_name):
724734
parser.add_argument(
725735
'--hidden',
726736
action='store_true',
737+
dest='is_hidden',
727738
default=False,
728739
help=_('List hidden images'),
729740
)
@@ -774,16 +785,8 @@ def take_action(self, parsed_args):
774785
image_client = self.app.client_manager.image
775786

776787
kwargs = {}
777-
if parsed_args.public:
778-
kwargs['visibility'] = 'public'
779-
if parsed_args.private:
780-
kwargs['visibility'] = 'private'
781-
if parsed_args.community:
782-
kwargs['visibility'] = 'community'
783-
if parsed_args.shared:
784-
kwargs['visibility'] = 'shared'
785-
if parsed_args.all:
786-
kwargs['visibility'] = 'all'
788+
if parsed_args.visibility is not None:
789+
kwargs['visibility'] = parsed_args.visibility
787790
if parsed_args.limit:
788791
kwargs['limit'] = parsed_args.limit
789792
if parsed_args.marker:
@@ -804,8 +807,8 @@ def take_action(self, parsed_args):
804807
parsed_args.project_domain,
805808
).id
806809
kwargs['owner'] = project_id
807-
if parsed_args.hidden:
808-
kwargs['is_hidden'] = True
810+
if parsed_args.is_hidden:
811+
kwargs['is_hidden'] = parsed_args.is_hidden
809812
if parsed_args.long:
810813
columns = (
811814
'ID',
@@ -1014,32 +1017,44 @@ def get_parser(self, prog_name):
10141017
protected_group.add_argument(
10151018
"--protected",
10161019
action="store_true",
1020+
dest="is_protected",
1021+
default=None,
10171022
help=_("Prevent image from being deleted"),
10181023
)
10191024
protected_group.add_argument(
10201025
"--unprotected",
1021-
action="store_true",
1026+
action="store_false",
1027+
dest="is_protected",
1028+
default=None,
10221029
help=_("Allow image to be deleted (default)"),
10231030
)
10241031
public_group = parser.add_mutually_exclusive_group()
10251032
public_group.add_argument(
10261033
"--public",
1027-
action="store_true",
1034+
action="store_const",
1035+
const="public",
1036+
dest="visibility",
10281037
help=_("Image is accessible to the public"),
10291038
)
10301039
public_group.add_argument(
10311040
"--private",
1032-
action="store_true",
1041+
action="store_const",
1042+
const="private",
1043+
dest="visibility",
10331044
help=_("Image is inaccessible to the public (default)"),
10341045
)
10351046
public_group.add_argument(
10361047
"--community",
1037-
action="store_true",
1048+
action="store_const",
1049+
const="community",
1050+
dest="visibility",
10381051
help=_("Image is accessible to the community"),
10391052
)
10401053
public_group.add_argument(
10411054
"--shared",
1042-
action="store_true",
1055+
action="store_const",
1056+
const="shared",
1057+
dest="visibility",
10431058
help=_("Image can be shared"),
10441059
)
10451060
parser.add_argument(
@@ -1120,7 +1135,7 @@ def get_parser(self, prog_name):
11201135
parser.add_argument(
11211136
"--%s" % deadopt,
11221137
metavar="<%s>" % deadopt,
1123-
dest=deadopt.replace('-', '_'),
1138+
dest=f"dead_{deadopt.replace('-', '_')}",
11241139
help=argparse.SUPPRESS,
11251140
)
11261141

@@ -1153,14 +1168,14 @@ def get_parser(self, prog_name):
11531168
hidden_group = parser.add_mutually_exclusive_group()
11541169
hidden_group.add_argument(
11551170
"--hidden",
1156-
dest='hidden',
1171+
dest="is_hidden",
11571172
default=None,
11581173
action="store_true",
11591174
help=_("Hide the image"),
11601175
)
11611176
hidden_group.add_argument(
11621177
"--unhidden",
1163-
dest='hidden',
1178+
dest="is_hidden",
11641179
default=None,
11651180
action="store_false",
11661181
help=_("Unhide the image"),
@@ -1172,7 +1187,7 @@ def take_action(self, parsed_args):
11721187
image_client = self.app.client_manager.image
11731188

11741189
for deadopt in self.deadopts:
1175-
if getattr(parsed_args, deadopt.replace('-', '_'), None):
1190+
if getattr(parsed_args, f"dead_{deadopt.replace('-', '_')}", None):
11761191
raise exceptions.CommandError(
11771192
_(
11781193
"ERROR: --%s was given, which is an Image v1 option"
@@ -1258,26 +1273,22 @@ def take_action(self, parsed_args):
12581273
# a single value for the pair of options because the default must be
12591274
# to do nothing when no options are present as opposed to always
12601275
# setting a default.
1261-
if parsed_args.protected:
1262-
kwargs['is_protected'] = True
1263-
if parsed_args.unprotected:
1264-
kwargs['is_protected'] = False
1265-
if parsed_args.public:
1266-
kwargs['visibility'] = 'public'
1267-
if parsed_args.private:
1268-
kwargs['visibility'] = 'private'
1269-
if parsed_args.community:
1270-
kwargs['visibility'] = 'community'
1271-
if parsed_args.shared:
1272-
kwargs['visibility'] = 'shared'
1276+
if parsed_args.is_protected is not None:
1277+
kwargs['is_protected'] = parsed_args.is_protected
1278+
1279+
if parsed_args.visibility is not None:
1280+
kwargs['visibility'] = parsed_args.visibility
1281+
12731282
if parsed_args.project:
12741283
# We already did the project lookup above
12751284
kwargs['owner_id'] = project_id
1285+
12761286
if parsed_args.tags:
12771287
# Tags should be extended, but duplicates removed
12781288
kwargs['tags'] = list(set(image.tags).union(set(parsed_args.tags)))
1279-
if parsed_args.hidden is not None:
1280-
kwargs['is_hidden'] = parsed_args.hidden
1289+
1290+
if parsed_args.is_hidden is not None:
1291+
kwargs['is_hidden'] = parsed_args.is_hidden
12811292

12821293
try:
12831294
image = image_client.update_image(image.id, **kwargs)

0 commit comments

Comments
 (0)