)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":17860,"name":"Samuel de Medeiros Queiroz","email":"samueldmq@gmail.com","username":"samueldmq"},"change_message_id":"b085b53cfe4d56fe8d37e98b8151a26e740f3681","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix mapping_purge failure"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This fixes the keystone-manage mapping_purge that fails silently."},{"line_number":10,"context_line":"This marks the required arguements as required\u003dTrue."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I01a814f03e83911461168f4dac74202b6f0537d8"},{"line_number":13,"context_line":"Closes-Bug: 1645568"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a71b18c_c5f7aff3","line":10,"range":{"start_line":10,"start_character":24,"end_line":10,"end_character":34},"updated":"2016-12-08 13:43:18.000000000","message":"(typo) arguments","commit_id":"c14ff9bf9673c96f6449e935f9610aee25b9bbf3"}],"keystone/cmd/cli.py":[{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"a81a2ce5ad1d9db5f923fcb7a224cbefbdb5b929","unresolved":false,"context_lines":[{"line_number":780,"context_line":"    name \u003d \u0027mapping_purge\u0027"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"},{"line_number":783,"context_line":"    def add_argument_parser(cls, subparsers):"},{"line_number":784,"context_line":"        parser \u003d super(MappingPurge, cls).add_argument_parser(subparsers)"},{"line_number":785,"context_line":"        parser.add_argument(\u0027--all\u0027, default\u003dFalse, action\u003d\u0027store_true\u0027,"},{"line_number":786,"context_line":"                            required\u003dTrue,"},{"line_number":787,"context_line":"                            help\u003d(\u0027Purge all mappings.\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a71b18c_9ff8b8c6","line":784,"range":{"start_line":783,"start_character":4,"end_line":784,"end_character":73},"updated":"2016-12-08 05:39:41.000000000","message":"The proposed implementation will just make *all* the options required, we want to make *one* of them required.\n\nI think we need something like this: https://github.com/openstack/python-openstackclient/blob/master/openstackclient/volume/v2/consistency_group.py#L86-L87\n\nso it\u0027ll probably look like:\n\n  parser \u003d super(MappingPurge, cls).add_argument_parser(subparsers)\n  exclusive_group \u003d parser.add_mutually_exclusive_group(required\u003dTrue)\n  exclusive_group.add_argument(\u0027--all\u0027, default\u003dFalse, action\u003d\u0027store_true\u0027,\n        ...\n  exclusive_group.add_argument(--domain-name, default\u003dNone,\n       ...\n\n and so forth.\n\nAdding the `required\u003dTrue` to the mutually exclusive group means that one of the options in the group will need to be specified.","commit_id":"c14ff9bf9673c96f6449e935f9610aee25b9bbf3"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"3c107ced0f7fc0862b5aa5f1670151663ce98050","unresolved":false,"context_lines":[{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"},{"line_number":783,"context_line":"    def add_argument_parser(cls, subparsers):"},{"line_number":784,"context_line":"        parser \u003d super(MappingPurge, cls).add_argument_parser(subparsers)"},{"line_number":785,"context_line":"        parser.add_argument(\u0027--all\u0027, default\u003dFalse, action\u003d\u0027store_true\u0027,"},{"line_number":786,"context_line":"                            required\u003dTrue,"},{"line_number":787,"context_line":"                            help\u003d(\u0027Purge all mappings.\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_3d7bd8f6","line":784,"in_reply_to":"3a71b18c_9ff8b8c6","updated":"2017-01-04 22:58:38.000000000","message":"Agreed - I think that would be a cleaner pattern.","commit_id":"c14ff9bf9673c96f6449e935f9610aee25b9bbf3"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"0f9f96a1e1ba331144d4b6f0739fbcf35c01ea0d","unresolved":false,"context_lines":[{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"},{"line_number":783,"context_line":"    def add_argument_parser(cls, subparsers):"},{"line_number":784,"context_line":"        parser \u003d super(MappingPurge, cls).add_argument_parser(subparsers)"},{"line_number":785,"context_line":"        parser.add_argument(\u0027--all\u0027, default\u003dFalse, action\u003d\u0027store_true\u0027,"},{"line_number":786,"context_line":"                            required\u003dTrue,"},{"line_number":787,"context_line":"                            help\u003d(\u0027Purge all mappings.\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_9f7ca3b6","line":784,"in_reply_to":"ba5201f7_3d7bd8f6","updated":"2017-01-05 13:21:47.000000000","message":"++ without Steve\u0027s suggestion the change is wrong.","commit_id":"c14ff9bf9673c96f6449e935f9610aee25b9bbf3"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"a81a2ce5ad1d9db5f923fcb7a224cbefbdb5b929","unresolved":false,"context_lines":[{"line_number":806,"context_line":"    @staticmethod"},{"line_number":807,"context_line":"    def main():"},{"line_number":808,"context_line":"        def validate_options():"},{"line_number":809,"context_line":"            # NOTE(henry-nash): It would be nice to use the argparse automated"},{"line_number":810,"context_line":"            # checking for this validation, but the only way I can see doing"},{"line_number":811,"context_line":"            # that is to make the default (i.e. if no optional parameters"},{"line_number":812,"context_line":"            # are specified) to purge all mappings - and that sounds too"},{"line_number":813,"context_line":"            # dangerous as a default.  So we use it in a slightly"},{"line_number":814,"context_line":"            # unconventional way, where all parameters are optional, but you"},{"line_number":815,"context_line":"            # must specify at least one."},{"line_number":816,"context_line":"            if (CONF.command.all is False and"},{"line_number":817,"context_line":"                CONF.command.domain_name is None and"},{"line_number":818,"context_line":"                CONF.command.public_id is None and"},{"line_number":819,"context_line":"                CONF.command.local_id is None and"},{"line_number":820,"context_line":"                    CONF.command.type is None):"},{"line_number":821,"context_line":"                raise ValueError(_(\u0027At least one option must be provided\u0027))"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"            if (CONF.command.all is True and"},{"line_number":824,"context_line":"                (CONF.command.domain_name is not None or"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a71b18c_bff574cc","line":821,"range":{"start_line":809,"start_character":0,"end_line":821,"end_character":75},"updated":"2016-12-08 05:39:41.000000000","message":"we can remove this guard if the proposed solution is implemented.","commit_id":"c14ff9bf9673c96f6449e935f9610aee25b9bbf3"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"3c107ced0f7fc0862b5aa5f1670151663ce98050","unresolved":false,"context_lines":[{"line_number":818,"context_line":"                CONF.command.public_id is None and"},{"line_number":819,"context_line":"                CONF.command.local_id is None and"},{"line_number":820,"context_line":"                    CONF.command.type is None):"},{"line_number":821,"context_line":"                raise ValueError(_(\u0027At least one option must be provided\u0027))"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"            if (CONF.command.all is True and"},{"line_number":824,"context_line":"                (CONF.command.domain_name is not None or"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_7d0ac07a","line":821,"in_reply_to":"3a71b18c_bff574cc","updated":"2017-01-04 22:58:38.000000000","message":"++","commit_id":"c14ff9bf9673c96f6449e935f9610aee25b9bbf3"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"f75e7021246da70544c530a1816cda59eb8d32a2","unresolved":false,"context_lines":[{"line_number":861,"context_line":"    @classmethod"},{"line_number":862,"context_line":"    def add_argument_parser(cls, subparsers):"},{"line_number":863,"context_line":"        parser \u003d super(MappingPurge, cls).add_argument_parser(subparsers)"},{"line_number":864,"context_line":"        exclusive_group \u003d parser.add_mutually_exclusive_group(required\u003dTrue)"},{"line_number":865,"context_line":"        exclusive_group.add_argument(\u0027--all\u0027, default\u003dFalse, action\u003d\u0027store_true\u0027,"},{"line_number":866,"context_line":"                            help\u003d(\u0027Purge all mappings.\u0027))"},{"line_number":867,"context_line":"        exclusive_group.add_argument(\u0027--domain-name\u0027, default\u003dNone,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a1ced50_81d99659","line":864,"range":{"start_line":864,"start_character":33,"end_line":864,"end_character":61},"updated":"2017-03-22 13:00:06.000000000","message":"very nice!","commit_id":"b0771f6f5c947224ed64ff7a1bf5979b222c42f9"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"f75e7021246da70544c530a1816cda59eb8d32a2","unresolved":false,"context_lines":[{"line_number":880,"context_line":""},{"line_number":881,"context_line":"    @staticmethod"},{"line_number":882,"context_line":"    def main():"},{"line_number":883,"context_line":"        def validate_options():"},{"line_number":884,"context_line":"            # NOTE(henry-nash): It would be nice to use the argparse automated"},{"line_number":885,"context_line":"            # checking for this validation, but the only way I can see doing"},{"line_number":886,"context_line":"            # that is to make the default (i.e. if no optional parameters"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a1ced50_e1232a54","line":883,"range":{"start_line":883,"start_character":0,"end_line":883,"end_character":31},"updated":"2017-03-22 13:00:06.000000000","message":"can this be removed?","commit_id":"b0771f6f5c947224ed64ff7a1bf5979b222c42f9"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"02d67857e3fea8e7bfe9947ece8d8c44449c2d01","unresolved":false,"context_lines":[{"line_number":881,"context_line":""},{"line_number":882,"context_line":"    @staticmethod"},{"line_number":883,"context_line":"    def main():"},{"line_number":884,"context_line":"        def validate_options():"},{"line_number":885,"context_line":"            # NOTE(henry-nash): It would be nice to use the argparse automated"},{"line_number":886,"context_line":"            # checking for this validation, but the only way I can see doing"},{"line_number":887,"context_line":"            # that is to make the default (i.e. if no optional parameters"},{"line_number":888,"context_line":"            # are specified) to purge all mappings - and that sounds too"},{"line_number":889,"context_line":"            # dangerous as a default.  So we use it in a slightly"},{"line_number":890,"context_line":"            # unconventional way, where all parameters are optional, but you"},{"line_number":891,"context_line":"            # must specify at least one."},{"line_number":892,"context_line":"            if (CONF.command.all is False and"},{"line_number":893,"context_line":"                CONF.command.domain_name is None and"},{"line_number":894,"context_line":"                CONF.command.public_id is None and"},{"line_number":895,"context_line":"                CONF.command.local_id is None and"},{"line_number":896,"context_line":"                    CONF.command.type is None):"},{"line_number":897,"context_line":"                raise ValueError(_(\u0027At least one option must be provided\u0027))"},{"line_number":898,"context_line":""},{"line_number":899,"context_line":"            if (CONF.command.all is True and"},{"line_number":900,"context_line":"                (CONF.command.domain_name is not None or"},{"line_number":901,"context_line":"                 CONF.command.public_id is not None or"},{"line_number":902,"context_line":"                 CONF.command.local_id is not None or"},{"line_number":903,"context_line":"                 CONF.command.type is not None)):"},{"line_number":904,"context_line":"                raise ValueError(_(\u0027--all option cannot be mixed with \u0027"},{"line_number":905,"context_line":"                                   \u0027other options\u0027))"},{"line_number":906,"context_line":""},{"line_number":907,"context_line":"        def get_domain_id(name):"},{"line_number":908,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_178e8dcf","line":905,"range":{"start_line":884,"start_character":0,"end_line":905,"end_character":52},"updated":"2017-04-08 13:08:24.000000000","message":"you can remove this logic now","commit_id":"745d87ca62ebfe8e51957c377aea0eec194d55f9"},{"author":{"_account_id":17860,"name":"Samuel de Medeiros Queiroz","email":"samueldmq@gmail.com","username":"samueldmq"},"change_message_id":"bc52e4829ca76d6657748f94442c51f68cc81d83","unresolved":false,"context_lines":[{"line_number":876,"context_line":"                                           \u0027ID specified.\u0027))"},{"line_number":877,"context_line":"        exclusive_group.add_argument(\u0027--type\u0027, default\u003dNone, choices\u003d[\u0027user\u0027,"},{"line_number":878,"context_line":"                                     \u0027group\u0027], help\u003d(\u0027Purge any mappings for \u0027"},{"line_number":879,"context_line":"                                     \u0027the type specified.\u0027))"},{"line_number":880,"context_line":"        return parser"},{"line_number":881,"context_line":""},{"line_number":882,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":10,"id":"df140735_bc7e49e1","line":879,"updated":"2017-06-06 01:44:18.000000000","message":"There could be a combination of --domain-name --public-id --local-id and --type, and now that\u0027s not possible anymore since they are all mutually exclusive","commit_id":"c6bcfad4ebd91baf7268e8a695c360de2f2dd8eb"},{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"89912df2c14b87f84409c3dd494ded863849dd62","unresolved":false,"context_lines":[{"line_number":876,"context_line":"                                           \u0027ID specified.\u0027))"},{"line_number":877,"context_line":"        exclusive_group.add_argument(\u0027--type\u0027, default\u003dNone, choices\u003d[\u0027user\u0027,"},{"line_number":878,"context_line":"                                     \u0027group\u0027], help\u003d(\u0027Purge any mappings for \u0027"},{"line_number":879,"context_line":"                                     \u0027the type specified.\u0027))"},{"line_number":880,"context_line":"        return parser"},{"line_number":881,"context_line":""},{"line_number":882,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":10,"id":"bf091321_14f03790","line":879,"in_reply_to":"df140735_bc7e49e1","updated":"2017-06-08 10:57:37.000000000","message":"I agree, we need to maintain the ability for multiple options. For instance, local_id by definition needs to have a domain identifier with it (e.g. --domain-name -local-id)","commit_id":"c6bcfad4ebd91baf7268e8a695c360de2f2dd8eb"},{"author":{"_account_id":26325,"name":"Omar Tleimat","email":"otleimat@ucla.edu","username":"otleimat"},"change_message_id":"6e94ffbf99bbe66f687938431b75a8f7afd30f3e","unresolved":false,"context_lines":[{"line_number":861,"context_line":"    @classmethod"},{"line_number":862,"context_line":"    def add_argument_parser(cls, subparsers):"},{"line_number":863,"context_line":"        parser \u003d super(MappingPurge, cls).add_argument_parser(subparsers)"},{"line_number":864,"context_line":"        group \u003d parser.add_argument_group(\u0027Options (At least one required)\u0027)"},{"line_number":865,"context_line":"        group.add_argument(\u0027--all\u0027, default\u003dFalse, action\u003d\u0027store_true\u0027,"},{"line_number":866,"context_line":"                           help\u003d(\u0027Purge all mappings. If chosen, no others \u0027"},{"line_number":867,"context_line":"                                 \u0027may be supplied.\u0027))"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_b2aa5acf","line":864,"range":{"start_line":864,"start_character":55,"end_line":864,"end_character":60},"updated":"2017-08-10 23:05:21.000000000","message":"\u0027keystone-manage mapping_purge --help\u0027 now produces: \n(see https://bugs.launchpad.net/keystone/+bug/1645568\nfor previous output)\n\nkeystone-manage mapping_purge --help\nusage: keystone-manage [bootstrap|credential_migrate|credential_rotate|credential_setup|db_sync|db_version|doctor|domain_config_upload|fernet_rotate|fernet_setup|mapping_populate|mapping_purge|mapping_engine|pki_setup|saml_idp_metadata|token_flush] mapping_purge\n       [-h] [--all] [--domain-name DOMAIN_NAME] [--public-id PUBLIC_ID]\n       [--local-id LOCAL_ID] [--type {user,group}]\n\noptional arguments:\n  -h, --help            show this help message and exit\n\nOptions (At least one required):\n  --all                 Purge all mappings. If chosen, no others may be\n                        supplied.\n  --domain-name DOMAIN_NAME\n                        Purge any mappings for the domain specified.\n  --public-id PUBLIC_ID\n                        Purge the mapping for the Public ID specified.\n  --local-id LOCAL_ID   Purge the mappings for the Local ID specified.\n  --type {user,group}   Purge any mappings for the type specified.","commit_id":"2758132da45247b2ed55183dda3157247759413d"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"2919f2ac904247393589f33598cda09c59b76bdb","unresolved":false,"context_lines":[{"line_number":913,"context_line":"        drivers \u003d backends.load_backends()"},{"line_number":914,"context_line":"        resource_manager \u003d drivers[\u0027resource_api\u0027]"},{"line_number":915,"context_line":"        mapping_manager \u003d drivers[\u0027id_mapping_api\u0027]"},{"line_number":916,"context_line":""},{"line_number":917,"context_line":"        # Now that we have validated the options, we know that at least one"},{"line_number":918,"context_line":"        # option has been specified, and if it was the --all option then this"},{"line_number":919,"context_line":"        # was the only option specified."}],"source_content_type":"text/x-python","patch_set":12,"id":"9f436f4f_bf43276c","side":"PARENT","line":916,"updated":"2017-08-11 17:12:13.000000000","message":"Add this back, unrelated change","commit_id":"c7e29560b7bf7a44e44722eea0645bf18ad56af3"},{"author":{"_account_id":26325,"name":"Omar Tleimat","email":"otleimat@ucla.edu","username":"otleimat"},"change_message_id":"4ca97f6809dc189884ebaf455dab02fbe81886af","unresolved":false,"context_lines":[{"line_number":913,"context_line":"        drivers \u003d backends.load_backends()"},{"line_number":914,"context_line":"        resource_manager \u003d drivers[\u0027resource_api\u0027]"},{"line_number":915,"context_line":"        mapping_manager \u003d drivers[\u0027id_mapping_api\u0027]"},{"line_number":916,"context_line":""},{"line_number":917,"context_line":"        # Now that we have validated the options, we know that at least one"},{"line_number":918,"context_line":"        # option has been specified, and if it was the --all option then this"},{"line_number":919,"context_line":"        # was the only option specified."}],"source_content_type":"text/x-python","patch_set":12,"id":"9f436f4f_459c6e69","side":"PARENT","line":916,"in_reply_to":"9f436f4f_bf43276c","updated":"2017-08-11 19:22:00.000000000","message":"Done","commit_id":"c7e29560b7bf7a44e44722eea0645bf18ad56af3"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"2919f2ac904247393589f33598cda09c59b76bdb","unresolved":false,"context_lines":[{"line_number":881,"context_line":""},{"line_number":882,"context_line":"    @staticmethod"},{"line_number":883,"context_line":"    def main():"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"        def validate_options():"},{"line_number":886,"context_line":"            # NOTE(henry-nash): It would be nice to use the argparse automated"},{"line_number":887,"context_line":"            # checking for this validation, but the only way I can see doing"}],"source_content_type":"text/x-python","patch_set":12,"id":"9f436f4f_5f755bca","line":884,"updated":"2017-08-11 17:12:13.000000000","message":"Unnecessary line break","commit_id":"c1f98260df60a943706684095ec6f3889ef398e7"},{"author":{"_account_id":26325,"name":"Omar Tleimat","email":"otleimat@ucla.edu","username":"otleimat"},"change_message_id":"4ca97f6809dc189884ebaf455dab02fbe81886af","unresolved":false,"context_lines":[{"line_number":881,"context_line":""},{"line_number":882,"context_line":"    @staticmethod"},{"line_number":883,"context_line":"    def main():"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"        def validate_options():"},{"line_number":886,"context_line":"            # NOTE(henry-nash): It would be nice to use the argparse automated"},{"line_number":887,"context_line":"            # checking for this validation, but the only way I can see doing"}],"source_content_type":"text/x-python","patch_set":12,"id":"9f436f4f_25996a78","line":884,"in_reply_to":"9f436f4f_5f755bca","updated":"2017-08-11 19:22:00.000000000","message":"Done","commit_id":"c1f98260df60a943706684095ec6f3889ef398e7"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"f1702d2c38d03319f244808cd9fcf5f5405c97f2","unresolved":false,"context_lines":[{"line_number":916,"context_line":"            if args.local_id is not None:"},{"line_number":917,"context_line":"                mapping[\u0027local_id\u0027] \u003d args.local_id"},{"line_number":918,"context_line":"            if args.type is not None:"},{"line_number":919,"context_line":"                mapping[\u0027type\u0027] \u003d args.type"},{"line_number":920,"context_line":"            return mapping"},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"        def get_domain_id(name):"}],"source_content_type":"text/x-python","patch_set":13,"id":"7f287b81_dd6c74cf","line":919,"updated":"2017-08-29 21:19:43.000000000","message":"Do these all have to be separate if statements or can they be consolidated using if/else? Having an elif will help denote different cases.","commit_id":"e45530fb771751a1347d350a6249c5857f24ecdb"},{"author":{"_account_id":26325,"name":"Omar Tleimat","email":"otleimat@ucla.edu","username":"otleimat"},"change_message_id":"7550490ca3439594f4c4ab899bcab2a1569905b6","unresolved":false,"context_lines":[{"line_number":916,"context_line":"            if args.local_id is not None:"},{"line_number":917,"context_line":"                mapping[\u0027local_id\u0027] \u003d args.local_id"},{"line_number":918,"context_line":"            if args.type is not None:"},{"line_number":919,"context_line":"                mapping[\u0027type\u0027] \u003d args.type"},{"line_number":920,"context_line":"            return mapping"},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"        def get_domain_id(name):"}],"source_content_type":"text/x-python","patch_set":13,"id":"7f287b81_f6298d36","line":919,"in_reply_to":"7f287b81_dd6c74cf","updated":"2017-08-29 23:18:26.000000000","message":"They should be all separate ifs to allow for different combinations. But, after talking to Colleen in IRC, I will revert this code and just adapt the unit tests. I\u0027ll upload a new patch tomorrow morning that also addresses your nit in the tests","commit_id":"e45530fb771751a1347d350a6249c5857f24ecdb"}],"keystone/tests/unit/test_cli.py":[{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"da35119e2e5138c1d0115b189727b2f5710476b2","unresolved":false,"context_lines":[{"line_number":1320,"context_line":"            tokens_fernet.symptom_usability_of_Fernet_key_repository())"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":""},{"line_number":1323,"context_line":"class TestMappingPurge(unit.SQLDriverOverrides, unit.TestCase):"},{"line_number":1324,"context_line":""},{"line_number":1325,"context_line":"    def test_add_argument_parser_with_all(self):"},{"line_number":1326,"context_line":"        parser_test \u003d argparse.ArgumentParser()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ffe62b97_8854b054","line":1323,"updated":"2017-03-28 03:48:18.000000000","message":"These tests are not exercising the 2 things that this patch addresses: 1. That the argument is required. 2. The mutual exclusivity of the arguments.","commit_id":"8fb7779bda536c9e0ee61743e38640ed7714dd7a"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"414ef3c283b955367589f247fe3ac4b7f904d4ce","unresolved":false,"context_lines":[{"line_number":1320,"context_line":"            tokens_fernet.symptom_usability_of_Fernet_key_repository())"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":""},{"line_number":1323,"context_line":"class TestMappingPurge(unit.SQLDriverOverrides, unit.TestCase):"},{"line_number":1324,"context_line":""},{"line_number":1325,"context_line":"    def test_add_argument_parser_with_all(self):"},{"line_number":1326,"context_line":"        parser_test \u003d argparse.ArgumentParser()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ffe62b97_b7f7be59","line":1323,"in_reply_to":"ffe62b97_8854b054","updated":"2017-03-28 13:12:26.000000000","message":"++\n\nyou should add tests that exercise the mutual exclusivity. Otherwise, this lgtm.","commit_id":"8fb7779bda536c9e0ee61743e38640ed7714dd7a"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"02d67857e3fea8e7bfe9947ece8d8c44449c2d01","unresolved":false,"context_lines":[{"line_number":1331,"context_line":"            options.domain_name is None and"},{"line_number":1332,"context_line":"            options.local_id is None and"},{"line_number":1333,"context_line":"            options.type is None):"},{"line_number":1334,"context_line":"            status \u003d \"Atleast one value must be present\"        "},{"line_number":1335,"context_line":"            self.assertRaises(ValueError,status)"},{"line_number":1336,"context_line":"            self.assertTrue(options.all)"},{"line_number":1337,"context_line":"            self.assertFalse(options.domain_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_d7c145e6","line":1334,"range":{"start_line":1334,"start_character":22,"end_line":1334,"end_character":29},"updated":"2017-04-08 13:08:24.000000000","message":"\"At least\"","commit_id":"745d87ca62ebfe8e51957c377aea0eec194d55f9"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"02d67857e3fea8e7bfe9947ece8d8c44449c2d01","unresolved":false,"context_lines":[{"line_number":1331,"context_line":"            options.domain_name is None and"},{"line_number":1332,"context_line":"            options.local_id is None and"},{"line_number":1333,"context_line":"            options.type is None):"},{"line_number":1334,"context_line":"            status \u003d \"Atleast one value must be present\"        "},{"line_number":1335,"context_line":"            self.assertRaises(ValueError,status)"},{"line_number":1336,"context_line":"            self.assertTrue(options.all)"},{"line_number":1337,"context_line":"            self.assertFalse(options.domain_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_f7be8963","line":1334,"range":{"start_line":1334,"start_character":56,"end_line":1334,"end_character":64},"updated":"2017-04-08 13:08:24.000000000","message":"remove the white space here","commit_id":"745d87ca62ebfe8e51957c377aea0eec194d55f9"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"91008e223b98ccc390e9c0b6048d4d50a894affc","unresolved":false,"context_lines":[{"line_number":1330,"context_line":"        if (options.all is False and"},{"line_number":1331,"context_line":"                options.domain_name is None and"},{"line_number":1332,"context_line":"                options.local_id is None and"},{"line_number":1333,"context_line":"                options.type is None):"},{"line_number":1334,"context_line":"            status \u003d \"At least one value must be present\""},{"line_number":1335,"context_line":"            self.assertRaises(ValueError, status)"},{"line_number":1336,"context_line":"            self.assertTrue(options.all)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bff0334d_a448ca86","line":1333,"updated":"2017-04-09 15:17:22.000000000","message":"I am confused about how these options are exercised - it looks like line 1329 will guarantee options.all to be true","commit_id":"70e1ac995903f3fc88e95ab23db628242d9c4111"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"962af4f346b52078ca85d9b657eb2d08eb4d7a40","unresolved":false,"context_lines":[{"line_number":1330,"context_line":"        if (options.all is False and"},{"line_number":1331,"context_line":"                options.domain_name is None and"},{"line_number":1332,"context_line":"                options.local_id is None and"},{"line_number":1333,"context_line":"                options.type is None):"},{"line_number":1334,"context_line":"            status \u003d \"At least one value must be present\""},{"line_number":1335,"context_line":"            self.assertRaises(ValueError, status)"},{"line_number":1336,"context_line":"            self.assertTrue(options.all)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7ffa3b31_bc4dc173","line":1333,"in_reply_to":"bff0334d_497d847d","updated":"2017-04-17 21:22:56.000000000","message":"You should test that the exception is raised when no options are set, then set each option individually, then try setting more than one option and test that an exception is raised. There\u0027s no need for a conditional unless you wanted to involve a loop somehow. The way this is written, these assertions will never be reached.","commit_id":"70e1ac995903f3fc88e95ab23db628242d9c4111"},{"author":{"_account_id":19690,"name":"Aqsa Malik","email":"aqsamm@gmail.com","username":"Aqsa"},"change_message_id":"ecd2c9a0c0f1b70b41c70d0bc00e53e6c025a600","unresolved":false,"context_lines":[{"line_number":1330,"context_line":"        if (options.all is False and"},{"line_number":1331,"context_line":"                options.domain_name is None and"},{"line_number":1332,"context_line":"                options.local_id is None and"},{"line_number":1333,"context_line":"                options.type is None):"},{"line_number":1334,"context_line":"            status \u003d \"At least one value must be present\""},{"line_number":1335,"context_line":"            self.assertRaises(ValueError, status)"},{"line_number":1336,"context_line":"            self.assertTrue(options.all)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bff0334d_497d847d","line":1333,"in_reply_to":"bff0334d_a448ca86","updated":"2017-04-10 13:04:41.000000000","message":"The idea is to check the conditions with single option enabled. I would appreciate any suggestion or workaround.","commit_id":"70e1ac995903f3fc88e95ab23db628242d9c4111"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"962af4f346b52078ca85d9b657eb2d08eb4d7a40","unresolved":false,"context_lines":[{"line_number":1332,"context_line":"                options.local_id is None and"},{"line_number":1333,"context_line":"                options.type is None):"},{"line_number":1334,"context_line":"            status \u003d \"At least one value must be present\""},{"line_number":1335,"context_line":"            self.assertRaises(ValueError, status)"},{"line_number":1336,"context_line":"            self.assertTrue(options.all)"},{"line_number":1337,"context_line":"            self.assertFalse(options.domain_name)"},{"line_number":1338,"context_line":"            self.assertFalse(options.local_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7ffa3b31_9c3a0596","line":1335,"updated":"2017-04-17 21:22:56.000000000","message":"assertRaises needs to be run on a function:\n\nhttps://docs.python.org/2/library/unittest.html#unittest.TestCase.assertRaises","commit_id":"70e1ac995903f3fc88e95ab23db628242d9c4111"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"962af4f346b52078ca85d9b657eb2d08eb4d7a40","unresolved":false,"context_lines":[{"line_number":1333,"context_line":"                options.type is None):"},{"line_number":1334,"context_line":"            status \u003d \"At least one value must be present\""},{"line_number":1335,"context_line":"            self.assertRaises(ValueError, status)"},{"line_number":1336,"context_line":"            self.assertTrue(options.all)"},{"line_number":1337,"context_line":"            self.assertFalse(options.domain_name)"},{"line_number":1338,"context_line":"            self.assertFalse(options.local_id)"},{"line_number":1339,"context_line":"            self.assertFalse(options.type)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7ffa3b31_5c447d1b","line":1339,"range":{"start_line":1336,"start_character":12,"end_line":1339,"end_character":42},"updated":"2017-04-17 21:22:56.000000000","message":"These aren\u0027t really necessary","commit_id":"70e1ac995903f3fc88e95ab23db628242d9c4111"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"3be20f8757879f7f8ef868cb9df26f0fa5b48d2b","unresolved":false,"context_lines":[{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":""},{"line_number":1323,"context_line":"class TestMappingPurge(unit.SQLDriverOverrides, unit.TestCase):"},{"line_number":1324,"context_line":""},{"line_number":1325,"context_line":"    def test_add_argument_parser_no_arguments(self):"},{"line_number":1326,"context_line":"        parser_test \u003d argparse.ArgumentParser()"},{"line_number":1327,"context_line":"        subparsers \u003d parser_test.add_subparsers()"}],"source_content_type":"text/x-python","patch_set":9,"id":"5ff73747_32d596ba","line":1324,"updated":"2017-04-22 15:11:59.000000000","message":"I think it would be fine to squash these cases all in one test_add_argument_parser test the way you had before, or alternately to use a setUp() method to put the common code in.","commit_id":"6b02e686f499c67f141b2e376b68b57e14d04c09"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"3be20f8757879f7f8ef868cb9df26f0fa5b48d2b","unresolved":false,"context_lines":[{"line_number":1343,"context_line":"        args \u003d ([\u0027--all\u0027])"},{"line_number":1344,"context_line":"        raised \u003d False"},{"line_number":1345,"context_line":"        try:"},{"line_number":1346,"context_line":"            parser.parse_args(args)"},{"line_number":1347,"context_line":"        except Exception:"},{"line_number":1348,"context_line":"            raised \u003d True"},{"line_number":1349,"context_line":"        self.assertFalse(raised, \u0027Exception raised\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5ff73747_72cb1e1e","line":1346,"range":{"start_line":1346,"start_character":12,"end_line":1346,"end_character":35},"updated":"2017-04-22 15:11:59.000000000","message":"You don\u0027t need the try/except, you could condense it to just this line - if it raises an unexpected exception then it fails the test, if it doesn\u0027t then it passes","commit_id":"6b02e686f499c67f141b2e376b68b57e14d04c09"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"7d119f394d13d3f7f935fdd1c5729069ca7fb06b","unresolved":false,"context_lines":[{"line_number":1327,"context_line":"        subparsers \u003d parser_test.add_subparsers()"},{"line_number":1328,"context_line":"        self.parser \u003d cli.MappingPurge.add_argument_parser(subparsers)"},{"line_number":1329,"context_line":"        args \u003d ([])"},{"line_number":1330,"context_line":"        self.assertRaises(unit.UnexpectedExit, self.parser.parse_args, args)"},{"line_number":1331,"context_line":"        args \u003d ([\u0027--all\u0027, \u0027--local-id\u0027, \u0027user\u0027])"},{"line_number":1332,"context_line":"        self.assertRaises(unit.UnexpectedExit, self.parser.parse_args, args)"},{"line_number":1333,"context_line":"        args \u003d ([\u0027--all\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"df140735_b0043216","line":1330,"updated":"2017-06-05 21:28:55.000000000","message":"We can isolate each of these cases into their own tests if we want to ease readability here. But, that\u0027s something we can do in a follow on patch.","commit_id":"c6bcfad4ebd91baf7268e8a695c360de2f2dd8eb"},{"author":{"_account_id":23659,"name":"Jaewoo Park","email":"jaewoopark91@gmail.com","username":"aselius"},"change_message_id":"02c40133de309f87d3b86938983a606ee92293b7","unresolved":false,"context_lines":[{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":""},{"line_number":1323,"context_line":"class TestMappingPurge(unit.BaseTestCase):"},{"line_number":1324,"context_line":"    class FakeConfCommand(object):"},{"line_number":1325,"context_line":""},{"line_number":1326,"context_line":"        def __init__(self, parent):"},{"line_number":1327,"context_line":"            self.extension \u003d False"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_c866382a","line":1324,"updated":"2017-08-11 15:59:31.000000000","message":"white space after test class","commit_id":"2758132da45247b2ed55183dda3157247759413d"},{"author":{"_account_id":23659,"name":"Jaewoo Park","email":"jaewoopark91@gmail.com","username":"aselius"},"change_message_id":"02c40133de309f87d3b86938983a606ee92293b7","unresolved":false,"context_lines":[{"line_number":1322,"context_line":""},{"line_number":1323,"context_line":"class TestMappingPurge(unit.BaseTestCase):"},{"line_number":1324,"context_line":"    class FakeConfCommand(object):"},{"line_number":1325,"context_line":""},{"line_number":1326,"context_line":"        def __init__(self, parent):"},{"line_number":1327,"context_line":"            self.extension \u003d False"},{"line_number":1328,"context_line":"            self.all \u003d parent.command_all"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_68b2ac96","line":1325,"updated":"2017-08-11 15:59:31.000000000","message":"unnecessary whitespace","commit_id":"2758132da45247b2ed55183dda3157247759413d"},{"author":{"_account_id":23659,"name":"Jaewoo Park","email":"jaewoopark91@gmail.com","username":"aselius"},"change_message_id":"02c40133de309f87d3b86938983a606ee92293b7","unresolved":false,"context_lines":[{"line_number":1336,"context_line":"        super(TestMappingPurge, self).setUp()"},{"line_number":1337,"context_line":"        self.config_fixture \u003d self.useFixture(oslo_config.fixture.Config(CONF))"},{"line_number":1338,"context_line":"        self.config_fixture.register_cli_opt(cli.command_opt)"},{"line_number":1339,"context_line":"        upgrades.offline_sync_database_to_version \u003d mock.Mock()"},{"line_number":1340,"context_line":"        upgrades.expand_schema \u003d mock.Mock()"},{"line_number":1341,"context_line":"        upgrades.migrate_data \u003d mock.Mock()"},{"line_number":1342,"context_line":"        upgrades.contract_schema \u003d mock.Mock()"},{"line_number":1343,"context_line":"        # For unit tests that should not throw any erorrs,"},{"line_number":1344,"context_line":"        # Use the argument parser to test that the combinations"},{"line_number":1345,"context_line":"        # work"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_c8c198e6","line":1342,"range":{"start_line":1339,"start_character":0,"end_line":1342,"end_character":46},"updated":"2017-08-11 15:59:31.000000000","message":"i dont think these are needed","commit_id":"2758132da45247b2ed55183dda3157247759413d"},{"author":{"_account_id":23659,"name":"Jaewoo Park","email":"jaewoopark91@gmail.com","username":"aselius"},"change_message_id":"02c40133de309f87d3b86938983a606ee92293b7","unresolved":false,"context_lines":[{"line_number":1346,"context_line":"        parser_test \u003d argparse.ArgumentParser()"},{"line_number":1347,"context_line":"        subparsers \u003d parser_test.add_subparsers()"},{"line_number":1348,"context_line":"        self.parser \u003d cli.MappingPurge.add_argument_parser(subparsers)"},{"line_number":1349,"context_line":""},{"line_number":1350,"context_line":"    def test_mapping_purge_no_arguments_fails(self):"},{"line_number":1351,"context_line":"        # Because custom logic had to be added in MappingPurge -"},{"line_number":1352,"context_line":"        # go through main() and make sure no arguments"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_887030de","line":1349,"updated":"2017-08-11 15:59:31.000000000","message":"Initialize the self.command s on the setup","commit_id":"2758132da45247b2ed55183dda3157247759413d"},{"author":{"_account_id":23659,"name":"Jaewoo Park","email":"jaewoopark91@gmail.com","username":"aselius"},"change_message_id":"02c40133de309f87d3b86938983a606ee92293b7","unresolved":false,"context_lines":[{"line_number":1356,"context_line":"        self.command_domain_name \u003d None"},{"line_number":1357,"context_line":"        self.command_local_id \u003d None"},{"line_number":1358,"context_line":"        self.command_public_id \u003d None"},{"line_number":1359,"context_line":"        self.useFixture(mockpatch.PatchObject("},{"line_number":1360,"context_line":"            CONF, \u0027command\u0027, self.FakeConfCommand(self)))"},{"line_number":1361,"context_line":"        self.assertRaises(ValueError, cli.MappingPurge.main)"},{"line_number":1362,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_488488b1","line":1359,"range":{"start_line":1359,"start_character":24,"end_line":1359,"end_character":33},"updated":"2017-08-11 15:59:31.000000000","message":"mockpatch is getting deprecated. replace with fixtures.MockPatchObject","commit_id":"2758132da45247b2ed55183dda3157247759413d"},{"author":{"_account_id":23659,"name":"Jaewoo Park","email":"jaewoopark91@gmail.com","username":"aselius"},"change_message_id":"02c40133de309f87d3b86938983a606ee92293b7","unresolved":false,"context_lines":[{"line_number":1369,"context_line":"        self.command_domain_name \u003d None"},{"line_number":1370,"context_line":"        self.command_local_id \u003d None"},{"line_number":1371,"context_line":"        self.command_public_id \u003d None"},{"line_number":1372,"context_line":"        self.useFixture(mockpatch.PatchObject("},{"line_number":1373,"context_line":"            CONF, \u0027command\u0027, self.FakeConfCommand(self)))"},{"line_number":1374,"context_line":"        self.assertRaises(ValueError, cli.MappingPurge.main)"},{"line_number":1375,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_487248e5","line":1372,"range":{"start_line":1372,"start_character":24,"end_line":1372,"end_character":33},"updated":"2017-08-11 15:59:31.000000000","message":"ditto","commit_id":"2758132da45247b2ed55183dda3157247759413d"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"f1702d2c38d03319f244808cd9fcf5f5405c97f2","unresolved":false,"context_lines":[{"line_number":1401,"context_line":"        args.append(\u0027test\u0027)"},{"line_number":1402,"context_line":"        self.parser.parse_args(args)"},{"line_number":1403,"context_line":"        args.append(\u0027--public-id\u0027)"},{"line_number":1404,"context_line":"        args.append(\u00271\u0027)"},{"line_number":1405,"context_line":"        self.parser.parse_args(args)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7f287b81_ab0ba127","line":1404,"updated":"2017-08-29 21:19:43.000000000","message":"nit: \n\n  args.append(uuid.uuid4().hex)\n\njust to show that the value of `1` in particular isn\u0027t significant.","commit_id":"e45530fb771751a1347d350a6249c5857f24ecdb"}]}