)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"bdfcae8d3b9f440e65fe8595e7b22e7efac2e71c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"81e86aca_2bde95d8","updated":"2022-07-07 12:26:48.000000000","message":"Agree with Chandan on setting the same default that we have on ansible module. Avoid future questions about this point again.","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"},{"author":{"_account_id":29775,"name":"Sandeep Yadav","email":"sandyada@redhat.com","username":"sandeepyadav93"},"change_message_id":"4777698792804dcbc48811111ddc6c80f695a9ea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"09c2c078_727ccd56","updated":"2022-07-05 06:11:12.000000000","message":"Looks good to me, but would be good to get eyes on this Patch from Arx/Chandan.","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"9987fde155c544d81d70b9fab8839361dba594ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cee8e803_3c7af982","updated":"2022-07-05 07:01:07.000000000","message":"looks right but comment inline. do we want that to be a required param? should we just default to \u0027all\u0027 ?","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"3e3e375eaf456b6def3412e445368824bdf64660","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"45a6b6dd_ca84c644","updated":"2022-07-07 19:13:19.000000000","message":"LGTM, thanks!","commit_id":"fc959e9f016f6572756d5c3efd802185da7781dc"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"b4476bff31fbf701b972ed2737cd52d3b418d494","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5ece592e_5bf13fb4","updated":"2022-07-08 07:19:07.000000000","message":"lgtm thanks for update ","commit_id":"fc959e9f016f6572756d5c3efd802185da7781dc"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"b1ffd95ee7da199d3b4d9a479f34a01bc3382fd8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b0a81ea0_5d1e70c9","updated":"2022-07-08 07:28:23.000000000","message":"thanks for considering the suggestion","commit_id":"fc959e9f016f6572756d5c3efd802185da7781dc"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"8adc0e0a988d25b92ecdde8fc45080e37f7db6fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d542460d_eab41cae","in_reply_to":"5ece592e_5bf13fb4","updated":"2022-07-08 07:19:49.000000000","message":"holding merge in case arx wanted to sanity check this (if he is out or there are no comments we can merge when you\u0027re in today dasm ping me?)","commit_id":"fc959e9f016f6572756d5c3efd802185da7781dc"}],"tempest_skip/list_yaml.py":[{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"9987fde155c544d81d70b9fab8839361dba594ee","unresolved":true,"context_lines":[{"line_number":51,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":52,"context_line":"                             if release[\u0027name\u0027] \u003d\u003d parsed_args.release]]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            tests \u003d [test for test in tests"},{"line_number":55,"context_line":"                     if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":56,"context_line":"                         if parsed_args.installer in"},{"line_number":57,"context_line":"                         release.get(\u0027installers\u0027, [\u0027tripleo\u0027, \u0027osp\u0027])]]"}],"source_content_type":"text/x-python","patch_set":1,"id":"780891e5_58c8df66","side":"PARENT","line":54,"updated":"2022-07-05 07:01:07.000000000","message":"its strange that only this one has no guard like the one you are adding.\n\ni looked and this came from https://opendev.org/openstack/openstack-tempest-skiplist/commit/85a392c55f55bd83a370c677c3174d567f288b1e so probably Arx who wrote that should have a look as Sandeep suggests","commit_id":"fcf64e7702e03c00cf89a4eca9c5d7517f7f024e"},{"author":{"_account_id":14611,"name":"dasm","display_name":"dasm","email":"dsmigiel@redhat.com","username":"dasm"},"change_message_id":"909eaf20110c6b4df7239ea780aaa19485a5fa76","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":52,"context_line":"                             if release[\u0027name\u0027] \u003d\u003d parsed_args.release]]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            tests \u003d [test for test in tests"},{"line_number":55,"context_line":"                     if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":56,"context_line":"                         if parsed_args.installer in"},{"line_number":57,"context_line":"                         release.get(\u0027installers\u0027, [\u0027tripleo\u0027, \u0027osp\u0027])]]"}],"source_content_type":"text/x-python","patch_set":1,"id":"6ddbd09d_26337d8d","side":"PARENT","line":54,"in_reply_to":"780891e5_58c8df66","updated":"2022-07-07 17:42:35.000000000","message":"Done","commit_id":"fcf64e7702e03c00cf89a4eca9c5d7517f7f024e"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"dab8653866387e0252d3b1f058cce65fb3c8a2c8","unresolved":true,"context_lines":[{"line_number":51,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":52,"context_line":"                             if release[\u0027name\u0027] \u003d\u003d parsed_args.release]]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            if parsed_args.installer:"},{"line_number":55,"context_line":"                tests \u003d [test for test in tests"},{"line_number":56,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":57,"context_line":"                             if parsed_args.installer in"}],"source_content_type":"text/x-python","patch_set":1,"id":"6e403457_3a2b7a72","line":54,"range":{"start_line":54,"start_character":11,"end_line":54,"end_character":37},"updated":"2022-07-05 16:48:20.000000000","message":"Not sure if this is the correct fix, or if we should provide a default value for \u0027installer\u0027. We need another opinion on that (from Arx?)","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"bdfcae8d3b9f440e65fe8595e7b22e7efac2e71c","unresolved":true,"context_lines":[{"line_number":51,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":52,"context_line":"                             if release[\u0027name\u0027] \u003d\u003d parsed_args.release]]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            if parsed_args.installer:"},{"line_number":55,"context_line":"                tests \u003d [test for test in tests"},{"line_number":56,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":57,"context_line":"                             if parsed_args.installer in"}],"source_content_type":"text/x-python","patch_set":1,"id":"f8f19710_df23fef7","line":54,"range":{"start_line":54,"start_character":11,"end_line":54,"end_character":37},"in_reply_to":"6e403457_3a2b7a72","updated":"2022-07-07 12:26:48.000000000","message":"Ok, I was thinking about that, and this looks as a proper fix to me. If there is another place that is setting a default parsed_args.installer value, it will work too. If parsed_args.installer is None, this list will continue to be empty here.\n\nAs Chadan mentioned in his comment on L87, the default came from the ansible module, so thats why we don\u0027t fail here, only when using the the cli:\nhttps://opendev.org/openstack/openstack-tempest-skiplist/src/branch/master/roles/list_skipped/library/list_skipped.py#L102\n\nShouldn\u0027t we set a default on CLI too? Beside this fix made here?","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"},{"author":{"_account_id":8367,"name":"Arx Cruz","email":"arxcruz@redhat.com","username":"arxcruz"},"change_message_id":"aaf63b4ec1957b94dd605e1547a713a632e9c1b9","unresolved":true,"context_lines":[{"line_number":51,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":52,"context_line":"                             if release[\u0027name\u0027] \u003d\u003d parsed_args.release]]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            if parsed_args.installer:"},{"line_number":55,"context_line":"                tests \u003d [test for test in tests"},{"line_number":56,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":57,"context_line":"                             if parsed_args.installer in"}],"source_content_type":"text/x-python","patch_set":1,"id":"f75ccc22_201d2453","line":54,"range":{"start_line":54,"start_character":11,"end_line":54,"end_character":37},"in_reply_to":"6e403457_3a2b7a72","updated":"2022-07-06 14:19:53.000000000","message":"The problem was because this was added after we have a ton of tests in the yaml file without the installer, so keep it as none, and if no installers was passed, return all. Maybe the logic isn\u0027t correct though.","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"9cff550fdca7533dcffcd23be265961b98f0166c","unresolved":true,"context_lines":[{"line_number":51,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":52,"context_line":"                             if release[\u0027name\u0027] \u003d\u003d parsed_args.release]]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            if parsed_args.installer:"},{"line_number":55,"context_line":"                tests \u003d [test for test in tests"},{"line_number":56,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":57,"context_line":"                             if parsed_args.installer in"}],"source_content_type":"text/x-python","patch_set":1,"id":"d7593cc7_8eb809e3","line":54,"range":{"start_line":54,"start_character":11,"end_line":54,"end_character":37},"in_reply_to":"f75ccc22_201d2453","updated":"2022-07-07 06:21:48.000000000","message":"+1 dviroel this is the same point i made below https://review.opendev.org/c/openstack/openstack-tempest-skiplist/+/848472/comment/51a708b7_b1ca5d07/","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"},{"author":{"_account_id":14611,"name":"dasm","display_name":"dasm","email":"dsmigiel@redhat.com","username":"dasm"},"change_message_id":"ffe9854f053c3af028713c86227b59b883a45e14","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":52,"context_line":"                             if release[\u0027name\u0027] \u003d\u003d parsed_args.release]]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            if parsed_args.installer:"},{"line_number":55,"context_line":"                tests \u003d [test for test in tests"},{"line_number":56,"context_line":"                         if [release for release in test.get(\u0027releases\u0027, [])"},{"line_number":57,"context_line":"                             if parsed_args.installer in"}],"source_content_type":"text/x-python","patch_set":1,"id":"2f8a2bd2_90cc325c","line":54,"range":{"start_line":54,"start_character":11,"end_line":54,"end_character":37},"in_reply_to":"f8f19710_df23fef7","updated":"2022-07-07 17:42:23.000000000","message":"Done","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"9987fde155c544d81d70b9fab8839361dba594ee","unresolved":true,"context_lines":[{"line_number":84,"context_line":"                            help\u003d\u0027List the tests to be skipped in the \u0027"},{"line_number":85,"context_line":"                                 \u0027given deployment\u0027)"},{"line_number":86,"context_line":"        parser.add_argument(\u0027--installer\u0027, dest\u003d\u0027installer\u0027,"},{"line_number":87,"context_line":"                            default\u003dNone, help\u003d\u0027Tests to be skipped for a \u0027"},{"line_number":88,"context_line":"                                                \u0027particular installer. Use \u0027"},{"line_number":89,"context_line":"                                                \u0027tripleo for upstream, and osp\u0027"},{"line_number":90,"context_line":"                                                \u0027 for downstream\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"51a708b7_b1ca5d07","line":87,"range":{"start_line":87,"start_character":28,"end_line":87,"end_character":36},"updated":"2022-07-05 07:01:07.000000000","message":"i wonder if this is meant to be a required parameter though.\nif i want to list the skipped tests I probably care if it is the ones for OSP or tripleo and if i want all then we can add \u0027all\u0027 as one of the acceptable values (or perhaps the default?)","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"9cff550fdca7533dcffcd23be265961b98f0166c","unresolved":true,"context_lines":[{"line_number":84,"context_line":"                            help\u003d\u0027List the tests to be skipped in the \u0027"},{"line_number":85,"context_line":"                                 \u0027given deployment\u0027)"},{"line_number":86,"context_line":"        parser.add_argument(\u0027--installer\u0027, dest\u003d\u0027installer\u0027,"},{"line_number":87,"context_line":"                            default\u003dNone, help\u003d\u0027Tests to be skipped for a \u0027"},{"line_number":88,"context_line":"                                                \u0027particular installer. Use \u0027"},{"line_number":89,"context_line":"                                                \u0027tripleo for upstream, and osp\u0027"},{"line_number":90,"context_line":"                                                \u0027 for downstream\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"85dea50d_30b8b024","line":87,"range":{"start_line":87,"start_character":28,"end_line":87,"end_character":36},"in_reply_to":"408e15b0_1c4c076d","updated":"2022-07-07 06:21:48.000000000","message":"sounds right","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"f953188c5340b85307eba1868b694c06782ab892","unresolved":true,"context_lines":[{"line_number":84,"context_line":"                            help\u003d\u0027List the tests to be skipped in the \u0027"},{"line_number":85,"context_line":"                                 \u0027given deployment\u0027)"},{"line_number":86,"context_line":"        parser.add_argument(\u0027--installer\u0027, dest\u003d\u0027installer\u0027,"},{"line_number":87,"context_line":"                            default\u003dNone, help\u003d\u0027Tests to be skipped for a \u0027"},{"line_number":88,"context_line":"                                                \u0027particular installer. Use \u0027"},{"line_number":89,"context_line":"                                                \u0027tripleo for upstream, and osp\u0027"},{"line_number":90,"context_line":"                                                \u0027 for downstream\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"408e15b0_1c4c076d","line":87,"range":{"start_line":87,"start_character":28,"end_line":87,"end_character":36},"in_reply_to":"51a708b7_b1ca5d07","updated":"2022-07-06 11:43:06.000000000","message":"If we take a look at ansible library code\nhttps://opendev.org/openstack/openstack-tempest-skiplist/src/branch/master/roles/list_skipped/library/list_skipped.py#L102\n```\n installer\u003ddict(type\u003d\u0027str\u0027, required\u003dFalse, default\u003d\u0027tripleo\u0027)\n```\ntripleo is used as a default.\nMay be we can change the default to tripleo at LIne 87.","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"},{"author":{"_account_id":14611,"name":"dasm","display_name":"dasm","email":"dsmigiel@redhat.com","username":"dasm"},"change_message_id":"ffe9854f053c3af028713c86227b59b883a45e14","unresolved":false,"context_lines":[{"line_number":84,"context_line":"                            help\u003d\u0027List the tests to be skipped in the \u0027"},{"line_number":85,"context_line":"                                 \u0027given deployment\u0027)"},{"line_number":86,"context_line":"        parser.add_argument(\u0027--installer\u0027, dest\u003d\u0027installer\u0027,"},{"line_number":87,"context_line":"                            default\u003dNone, help\u003d\u0027Tests to be skipped for a \u0027"},{"line_number":88,"context_line":"                                                \u0027particular installer. Use \u0027"},{"line_number":89,"context_line":"                                                \u0027tripleo for upstream, and osp\u0027"},{"line_number":90,"context_line":"                                                \u0027 for downstream\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"a422887f_6b21b955","line":87,"range":{"start_line":87,"start_character":28,"end_line":87,"end_character":36},"in_reply_to":"85dea50d_30b8b024","updated":"2022-07-07 17:42:23.000000000","message":"Done","commit_id":"502addd4fd2b621694a05e4b4da4242cb1ab918d"}]}
