)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"08824be2c1a5f8b865f25d874f6618e3b659f0d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"95c47b06_ca267b5e","updated":"2026-06-23 15:45:36.000000000","message":"I did an initial review and test in a test deployment. The most notable issue i found was related to lack of api version validation in patch calls to audittemplate. I still need to do a second review of the unit tests coverage.\n\nAbout documentation, audit templates may deserve a more clear documentation about how templates and audits work. i.e. iiuc the audit template is only applied to the audit at creation of the audit. If an audittemplate is patched, that does not affect to the ongoing continuous or event audits created from it previously (am I correct?). Actually, current description in https://docs.openstack.org/watcher/latest/glossary.html#audit-template may deserve some review.\n\nAlso, a sentence stating that default_parameters from audittemplates will be merged with the audit parameters when creating from audittemplates may be usefull in the audit create documentation https://github.com/openstack/watcher/blob/master/api-ref/source/watcher-api-v1-audits.inc?plain\u003d1#L27","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"ad430f18da737abd91fd67a373e5f2748e7dc1d7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ee5aaf6c_73c81a0b","updated":"2026-06-16 22:42:55.000000000","message":"recheck\n\nmaybe we need to increase jobs timeout","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"96a4b4d0e06a9fbf9cab8d9c1b375a706de5dcb1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7eb32a32_11b5c73b","updated":"2026-06-18 18:06:54.000000000","message":"recheck\n\nrelease notes failure not related to this patch","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"b2cc3d1e783c3542d83b4d5ef5029d405faa55fd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f0d0888a_496892ea","updated":"2026-06-22 17:22:23.000000000","message":"teim-ci: manual","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"0935bcff1c08e14fdbbe9c68f5ce63dba0fdc932","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"dfa07bcd_6314ea9f","in_reply_to":"95c47b06_ca267b5e","updated":"2026-06-23 15:52:03.000000000","message":"Thanks Alfredo. Ouch for the missing validation and +1 on updating the docs about this new parameter and behavior for Audits, totally makes sense, i  will fix them in the next ps!","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"2d313f3c024e1ae19c77d73cfc125c578f436887","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a61e788b_b678f218","in_reply_to":"dfa07bcd_6314ea9f","updated":"2026-06-26 18:11:04.000000000","message":"Changed a bit how this was built. Both Post and Patch are now validating the microversion when default_parameters is provided.\nThey also validate if the default_parametes match the schema, in a more similar way, in the controller.","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f0a7cd99a6d6935ba0c8f1f4ef74108bbacd346b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b92d7d91_d0fa12a9","updated":"2026-07-01 11:36:39.000000000","message":"Thanks for raising this issue Alfredo, I will work on a fix soon!","commit_id":"3c9c9f5302a4a8b3e91caf2072c27b53a6744518"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"893e4bffd71a0567e174b3712568a435ab6653f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e2f4ba51_52b1d2cf","updated":"2026-07-01 10:02:02.000000000","message":"Thanks for the changes. Docs look good and additional testing coverage. I hit an issue related to parameters validation mutating the default_parameters, which i think is undesired. See my comments inline. Other than that, i think looks fine.","commit_id":"3c9c9f5302a4a8b3e91caf2072c27b53a6744518"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"7ff7ee79128f5cdcec9c1435b57f759493fbd033","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4b270683_23cd270b","updated":"2026-07-27 08:27:42.000000000","message":"Thanks for your improvements. I\u0027ve also tested locally and all use cases worked for me.","commit_id":"105d7324e4eb7b3c32c3c44355daa0d43be3f517"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ee58f6ed1abd04b844ac6ae1891a7300621bbf27","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"0460878e_db784676","updated":"2026-07-27 10:26:29.000000000","message":"lgtm","commit_id":"105d7324e4eb7b3c32c3c44355daa0d43be3f517"}],"doc/source/user/user-guide.rst":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"893e4bffd71a0567e174b3712568a435ab6653f9","unresolved":true,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":".. note::"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"   Changes to ``default_parameters`` on an Audit Template do **not**"},{"line_number":135,"context_line":"   cascade to Audits that were already created from it. Each Audit captures"},{"line_number":136,"context_line":"   the effective parameters at creation time and is not affected by"},{"line_number":137,"context_line":"   subsequent template edits."},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"Then, you can create an audit. An audit is a request for optimizing your"},{"line_number":140,"context_line":"cluster depending on the specified :ref:`goal \u003cgoal_definition\u003e`."}],"source_content_type":"text/x-rst","patch_set":3,"id":"b7a29df2_43c69528","line":137,"range":{"start_line":134,"start_character":0,"end_line":137,"end_character":29},"updated":"2026-07-01 10:02:02.000000000","message":"+1. Thanks for the clear explanation","commit_id":"3c9c9f5302a4a8b3e91caf2072c27b53a6744518"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"c4126395c018ab8663060c47fc0dbe9866847b84","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":".. note::"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"   Changes to ``default_parameters`` on an Audit Template do **not**"},{"line_number":135,"context_line":"   cascade to Audits that were already created from it. Each Audit captures"},{"line_number":136,"context_line":"   the effective parameters at creation time and is not affected by"},{"line_number":137,"context_line":"   subsequent template edits."},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"Then, you can create an audit. An audit is a request for optimizing your"},{"line_number":140,"context_line":"cluster depending on the specified :ref:`goal \u003cgoal_definition\u003e`."}],"source_content_type":"text/x-rst","patch_set":3,"id":"714792af_216c7801","line":137,"range":{"start_line":134,"start_character":0,"end_line":137,"end_character":29},"in_reply_to":"b7a29df2_43c69528","updated":"2026-07-01 13:26:09.000000000","message":"Done","commit_id":"3c9c9f5302a4a8b3e91caf2072c27b53a6744518"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"4e6e41cfcfe9ad4d62b0c4cff446557e6050c054","unresolved":false,"context_lines":[{"line_number":165,"context_line":"  $ openstack optimize audit create -a \u003cyour_audit_template\u003e \\"},{"line_number":166,"context_line":"    -p \u003cyour_strategy_para1\u003e\u003d5.5 -p \u003cyour_strategy_para2\u003e\u003dhi"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"If the Audit Template has ``default_parameters`` configured, new Audits"},{"line_number":169,"context_line":"created from it will inherit those values automatically. You do not need to"},{"line_number":170,"context_line":"supply ``-p`` unless you want to override a specific parameter:"},{"line_number":171,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"9075eee6_2176413b","line":168,"updated":"2026-07-20 13:58:52.000000000","message":"+1 for calling this out, I was unsure about the behaviour here, so having this documented is great","commit_id":"49645b83d8a1e4a9c2ff5595476740fba2a7069a"}],"watcher/api/controllers/v1/audit.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"2d313f3c024e1ae19c77d73cfc125c578f436887","unresolved":true,"context_lines":[{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    force \u003d wtypes.wsattr(bool, mandatory\u003dFalse)"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def as_audit(self, context):"},{"line_number":119,"context_line":"        audit_type_values \u003d [val.value for val in objects.audit.AuditType]"},{"line_number":120,"context_line":"        if self.audit_type not in audit_type_values:"},{"line_number":121,"context_line":"            raise exception.AuditTypeNotFound(audit_type\u003dself.audit_type)"}],"source_content_type":"text/x-python","patch_set":2,"id":"35639b19_34db92c4","side":"PARENT","line":118,"range":{"start_line":118,"start_character":8,"end_line":118,"end_character":16},"updated":"2026-06-26 18:11:04.000000000","message":"due to many if-else within the same method, as_audit has to be broken into smaller private methods.","commit_id":"5431c5af187b2570649d45d8c8b7f85ce2c92498"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"893e4bffd71a0567e174b3712568a435ab6653f9","unresolved":true,"context_lines":[{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    force \u003d wtypes.wsattr(bool, mandatory\u003dFalse)"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def as_audit(self, context):"},{"line_number":119,"context_line":"        audit_type_values \u003d [val.value for val in objects.audit.AuditType]"},{"line_number":120,"context_line":"        if self.audit_type not in audit_type_values:"},{"line_number":121,"context_line":"            raise exception.AuditTypeNotFound(audit_type\u003dself.audit_type)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f166dd67_16929359","side":"PARENT","line":118,"range":{"start_line":118,"start_character":8,"end_line":118,"end_character":16},"in_reply_to":"35639b19_34db92c4","updated":"2026-07-01 10:02:02.000000000","message":"Thanks, I think it\u0027s better now.","commit_id":"5431c5af187b2570649d45d8c8b7f85ce2c92498"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"c4126395c018ab8663060c47fc0dbe9866847b84","unresolved":false,"context_lines":[{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    force \u003d wtypes.wsattr(bool, mandatory\u003dFalse)"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def as_audit(self, context):"},{"line_number":119,"context_line":"        audit_type_values \u003d [val.value for val in objects.audit.AuditType]"},{"line_number":120,"context_line":"        if self.audit_type not in audit_type_values:"},{"line_number":121,"context_line":"            raise exception.AuditTypeNotFound(audit_type\u003dself.audit_type)"}],"source_content_type":"text/x-python","patch_set":2,"id":"89c01bbd_bc275036","side":"PARENT","line":118,"range":{"start_line":118,"start_character":8,"end_line":118,"end_character":16},"in_reply_to":"f166dd67_16929359","updated":"2026-07-01 13:26:09.000000000","message":"Done","commit_id":"5431c5af187b2570649d45d8c8b7f85ce2c92498"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:manual-ci","change_message_id":"f2b44c112eeaa347e1ec0fcc524a35df6f4a1e47","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                _(\u0027A valid goal or audit_template_id must be provided\u0027)"},{"line_number":126,"context_line":"            )"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        if self.audit_template_uuid and self.goal:"},{"line_number":129,"context_line":"            raise exception.Invalid("},{"line_number":130,"context_line":"                \u0027Either audit_template_uuid or goal should be provided.\u0027"},{"line_number":131,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":2,"id":"bc1f6b70_2c67813b","line":128,"updated":"2026-06-22 17:47:05.000000000","message":"Validation ordering change: the \u0027audit_template_uuid and goal\u0027 conflict check was relocated from after the ONESHOT/CONTINUOUS interval checks to before them. A multi-rule-invalid request now surfaces the template/goal conflict (Invalid) instead of the interval error.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Clients that relied on the previous exception type/message for a multi-rule-invalid request will see a different error. Pure correctness is preserved (both states are still rejected), but the observable error for ambiguous inputs changed within this patch.\n\n**Suggestion**:\nEither keep the original ordering (interval checks before the template/goal conflict check) to preserve existing error precedence, or document the new precedence in the REST API version history entry for 1.7 so consumers are aware.","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"6cac6b957dfafab79a706f09809b1f9b6cb920b5","unresolved":false,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        if self.audit_template_uuid and self.goal:"},{"line_number":129,"context_line":"            raise exception.Invalid("},{"line_number":130,"context_line":"                \u0027Either audit_template_uuid or goal should be provided.\u0027"},{"line_number":131,"context_line":"            )"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        if ("}],"source_content_type":"text/x-python","patch_set":4,"id":"e1646837_e08dd9e2","line":130,"updated":"2026-07-01 13:43:57.000000000","message":"The error string \u0027Either audit_template_uuid or goal should be provided.\u0027 is not wrapped in _() for translation. The refactor relocated this block (the check now runs earlier), so the line is in the diff. Adjacent error messages in the same method use _(); this one does not.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Keeps the new method consistent with Watcher\u0027s i18n policy (hacking checks N340/N341 require _() from watcher._i18n) and the sibling messages already wrapped in the same function.\n\n**Recommendation**:\nWrap the string: raise exception.Invalid(_(\u0027Either audit_template_uuid or goal should be provided.\u0027)).","commit_id":"589b9a1a1eaac96980cfd827ee954a9e9348eb11"}],"watcher/api/controllers/v1/audit_template.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:manual-ci","change_message_id":"f2b44c112eeaa347e1ec0fcc524a35df6f4a1e47","unresolved":false,"context_lines":[{"line_number":423,"context_line":"    scope \u003d wtypes.wsattr(types.jsontype, mandatory\u003dFalse)"},{"line_number":424,"context_line":"    \"\"\"Audit Scope\"\"\""},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"    default_parameters \u003d wtypes.wsattr(types.jsontype, mandatory\u003dFalse)"},{"line_number":427,"context_line":"    \"\"\"Default strategy parameters for audits created from this template\"\"\""},{"line_number":428,"context_line":""},{"line_number":429,"context_line":"    def __init__(self, **kwargs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"18fbb985_708578bd","line":426,"updated":"2026-06-22 17:47:05.000000000","message":"default_parameters WSME type differs between request and response: AuditTemplatePostType uses {wtypes.text: types.jsontype} (dict-typed) while the AuditTemplate response uses types.jsontype (free JSON). The existing Audit.parameters uses the dict-typed form on both.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Using {wtypes.text: types.jsontype} consistently for default_parameters (matching Audit.parameters) gives stricter request/response validation of the dict shape and aligns with the established pattern in the same controller, improving API contract clarity.\n\n**Recommendation**:\nChange the AuditTemplate.default_parameters declaration at line 426 from types.jsontype to {wtypes.text: types.jsontype} to mirror the POST type at line 99 and the existing Audit.parameters field.","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"08824be2c1a5f8b865f25d874f6618e3b659f0d9","unresolved":true,"context_lines":[{"line_number":777,"context_line":"    @wsme_pecan.wsexpose("},{"line_number":778,"context_line":"        AuditTemplate, wtypes.text, body\u003d[AuditTemplatePatchType]"},{"line_number":779,"context_line":"    )"},{"line_number":780,"context_line":"    def patch(self, audit_template, patch):"},{"line_number":781,"context_line":"        \"\"\"Update an existing audit template."},{"line_number":782,"context_line":""},{"line_number":783,"context_line":"        :param template_uuid: UUID of a audit template."}],"source_content_type":"text/x-python","patch_set":2,"id":"9d8d3767_9b973338","line":780,"range":{"start_line":780,"start_character":0,"end_line":780,"end_character":2},"updated":"2026-06-23 15:45:36.000000000","message":"it\u0027s not validating api version in PATCH call so it allows to patch default_parameters with version \u003c 1.7.","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"08824be2c1a5f8b865f25d874f6618e3b659f0d9","unresolved":true,"context_lines":[{"line_number":838,"context_line":"        new_default_params \u003d getattr("},{"line_number":839,"context_line":"            audit_template, \u0027default_parameters\u0027, None"},{"line_number":840,"context_line":"        )"},{"line_number":841,"context_line":"        if new_default_params not in (wtypes.Unset, None, {}):"},{"line_number":842,"context_line":"            strategy_id \u003d getattr(audit_template, \u0027strategy_id\u0027, None)"},{"line_number":843,"context_line":"            if strategy_id in (wtypes.Unset, None):"},{"line_number":844,"context_line":"                raise exception.Invalid("},{"line_number":845,"context_line":"                    _(\u0027default_parameters requires a strategy to be specified\u0027)"},{"line_number":846,"context_line":"                )"},{"line_number":847,"context_line":"            strategy \u003d objects.Strategy.get_by_id("},{"line_number":848,"context_line":"                pecan.request.context, strategy_id"},{"line_number":849,"context_line":"            )"},{"line_number":850,"context_line":"            schema \u003d strategy.parameters_spec"},{"line_number":851,"context_line":"            if schema:"},{"line_number":852,"context_line":"                try:"},{"line_number":853,"context_line":"                    common_utils.StrictDefaultValidatingDraft4Validator("},{"line_number":854,"context_line":"                        schema"},{"line_number":855,"context_line":"                    ).validate(new_default_params)"},{"line_number":856,"context_line":"                except jsonschema.exceptions.ValidationError as e:"},{"line_number":857,"context_line":"                    raise exception.Invalid("},{"line_number":858,"context_line":"                        _(\u0027Invalid default_parameters for strategy: %s\u0027) % e"},{"line_number":859,"context_line":"                    )"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":"        # Update only the fields that have changed"},{"line_number":862,"context_line":"        for field in objects.AuditTemplate.fields:"},{"line_number":863,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"2f5dfa42_2db70427","line":860,"range":{"start_line":841,"start_character":0,"end_line":860,"end_character":1},"updated":"2026-06-23 15:45:36.000000000","message":"This is duplicated with the validation code in post validation, may it be moved to a shared function?.\n\nActually, it\u0027d be nice if we could validate the patched version with the same code as a new one to avoid allowing invalid combination. i.e. currently patch calls allow to set a strategy for a different goal that the one in the audittemplate (this is pre-existing bug).","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"2d313f3c024e1ae19c77d73cfc125c578f436887","unresolved":true,"context_lines":[{"line_number":838,"context_line":"        new_default_params \u003d getattr("},{"line_number":839,"context_line":"            audit_template, \u0027default_parameters\u0027, None"},{"line_number":840,"context_line":"        )"},{"line_number":841,"context_line":"        if new_default_params not in (wtypes.Unset, None, {}):"},{"line_number":842,"context_line":"            strategy_id \u003d getattr(audit_template, \u0027strategy_id\u0027, None)"},{"line_number":843,"context_line":"            if strategy_id in (wtypes.Unset, None):"},{"line_number":844,"context_line":"                raise exception.Invalid("},{"line_number":845,"context_line":"                    _(\u0027default_parameters requires a strategy to be specified\u0027)"},{"line_number":846,"context_line":"                )"},{"line_number":847,"context_line":"            strategy \u003d objects.Strategy.get_by_id("},{"line_number":848,"context_line":"                pecan.request.context, strategy_id"},{"line_number":849,"context_line":"            )"},{"line_number":850,"context_line":"            schema \u003d strategy.parameters_spec"},{"line_number":851,"context_line":"            if schema:"},{"line_number":852,"context_line":"                try:"},{"line_number":853,"context_line":"                    common_utils.StrictDefaultValidatingDraft4Validator("},{"line_number":854,"context_line":"                        schema"},{"line_number":855,"context_line":"                    ).validate(new_default_params)"},{"line_number":856,"context_line":"                except jsonschema.exceptions.ValidationError as e:"},{"line_number":857,"context_line":"                    raise exception.Invalid("},{"line_number":858,"context_line":"                        _(\u0027Invalid default_parameters for strategy: %s\u0027) % e"},{"line_number":859,"context_line":"                    )"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":"        # Update only the fields that have changed"},{"line_number":862,"context_line":"        for field in objects.AuditTemplate.fields:"},{"line_number":863,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"0cdc934d_b38bc19b","line":860,"range":{"start_line":841,"start_character":0,"end_line":860,"end_character":1},"in_reply_to":"2f5dfa42_2db70427","updated":"2026-06-26 18:11:04.000000000","message":"moved the common spec check to _validate_default_parameters in the controller classe. It will only validate after microversion check in post/patch methods.\nRight, it seems that Patch only validates that Goal or Strategy exists, but do not validate that a Strategy matches with the Goal set of being updated. I believe that this needs a LP and a fix in another patch. Thanks!","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"893e4bffd71a0567e174b3712568a435ab6653f9","unresolved":true,"context_lines":[{"line_number":542,"context_line":"        schema \u003d strategy.parameters_spec"},{"line_number":543,"context_line":"        if schema:"},{"line_number":544,"context_line":"            try:"},{"line_number":545,"context_line":"                common_utils.StrictDefaultValidatingDraft4Validator("},{"line_number":546,"context_line":"                    schema"},{"line_number":547,"context_line":"                ).validate(default_parameters)"},{"line_number":548,"context_line":"            except jsonschema.exceptions.ValidationError as e:"}],"source_content_type":"text/x-python","patch_set":3,"id":"085a82a7_1138fae9","line":545,"updated":"2026-07-01 10:02:02.000000000","message":"This is having a side-effect. StrictDefaultValidatingDraft4Validator mutates the passed default_parameters by adding defaults https://github.com/openstack/watcher/blob/master/watcher/common/utils.py#L156-L157, so when I create or patch an AT with `\"default_parameters\": {\"para3\": \"5\"}` for dummy strategy, it saves `\"default_parameters\": {\"para1\": 5, \"para2\": \"hello\"}` in the database. I think that behavior is incorrect. \n\nI think using `extend_with_strict_schema(validators.Draft4Validator)(schema).validate(default_parameters)` instead of using StrictDefaultValidatingDraft4Validator would work. Alternatively, we may validate a copy of the actual default_attributes parameter in the AT instead.\n\nIt\u0027d be good to validate this in unit tests by creating a strategy with two params, creating default_params with only one and checking the resulting AT has only that one saved.","commit_id":"3c9c9f5302a4a8b3e91caf2072c27b53a6744518"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f0a7cd99a6d6935ba0c8f1f4ef74108bbacd346b","unresolved":true,"context_lines":[{"line_number":542,"context_line":"        schema \u003d strategy.parameters_spec"},{"line_number":543,"context_line":"        if schema:"},{"line_number":544,"context_line":"            try:"},{"line_number":545,"context_line":"                common_utils.StrictDefaultValidatingDraft4Validator("},{"line_number":546,"context_line":"                    schema"},{"line_number":547,"context_line":"                ).validate(default_parameters)"},{"line_number":548,"context_line":"            except jsonschema.exceptions.ValidationError as e:"}],"source_content_type":"text/x-python","patch_set":3,"id":"33da53a8_815fd7f2","line":545,"in_reply_to":"085a82a7_1138fae9","updated":"2026-07-01 11:36:39.000000000","message":"Good catch! you are correct, it returns a modified schema and it seems that this is the expected behavior in other places, but does not work for us here. IMO it is not correct that a \"validate\" method mutates the parameters like that. (it is more a validate_and_extend).\nI will think in a way to fix this issue but we\u0027ll still need to extend the parameters in order to validate the schema, since some of them are required by the schema, so we can\u0027t just skip the \u0027extend_with_default\u0027.","commit_id":"3c9c9f5302a4a8b3e91caf2072c27b53a6744518"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"c4126395c018ab8663060c47fc0dbe9866847b84","unresolved":true,"context_lines":[{"line_number":542,"context_line":"        schema \u003d strategy.parameters_spec"},{"line_number":543,"context_line":"        if schema:"},{"line_number":544,"context_line":"            try:"},{"line_number":545,"context_line":"                common_utils.StrictDefaultValidatingDraft4Validator("},{"line_number":546,"context_line":"                    schema"},{"line_number":547,"context_line":"                ).validate(default_parameters)"},{"line_number":548,"context_line":"            except jsonschema.exceptions.ValidationError as e:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9390d4b5_810a40dc","line":545,"in_reply_to":"33da53a8_815fd7f2","updated":"2026-07-01 13:26:09.000000000","message":"So, my proposal here is to provide a copy here and do not modify anything in StrictDefaultValidatingDraft4Validator, because 1) other places expect that behavior and 2) if we are going to touch these methods it will be good to also update this old schema validation (which has a LP open for it), but that\u0027s something for another change/fix. Let me know what you think about","commit_id":"3c9c9f5302a4a8b3e91caf2072c27b53a6744518"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"900fb9e596e84772cedf6b76de19a52fe9bff675","unresolved":true,"context_lines":[{"line_number":542,"context_line":"        schema \u003d strategy.parameters_spec"},{"line_number":543,"context_line":"        if schema:"},{"line_number":544,"context_line":"            try:"},{"line_number":545,"context_line":"                common_utils.StrictDefaultValidatingDraft4Validator("},{"line_number":546,"context_line":"                    schema"},{"line_number":547,"context_line":"                ).validate(default_parameters)"},{"line_number":548,"context_line":"            except jsonschema.exceptions.ValidationError as e:"}],"source_content_type":"text/x-python","patch_set":3,"id":"b0a748b6_97e946c2","line":545,"in_reply_to":"9390d4b5_810a40dc","updated":"2026-07-01 13:58:47.000000000","message":"+1, i wouldn\u0027t modify the StrictDefaultValidatingDraft4Validator if we have a feasible alternative, and passing a copy seems good enough","commit_id":"3c9c9f5302a4a8b3e91caf2072c27b53a6744518"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"b7ee664dc58123e1b0e367171c089bb62a64ed6a","unresolved":false,"context_lines":[{"line_number":761,"context_line":"        ):"},{"line_number":762,"context_line":"            raise exception.NotAcceptable()"},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"        context \u003d pecan.request.context"},{"line_number":765,"context_line":"        strategy_obj \u003d None"},{"line_number":766,"context_line":"        if audit_template_postdata.strategy:"},{"line_number":767,"context_line":"            strategy_obj \u003d objects.Strategy.get("}],"source_content_type":"text/x-python","patch_set":3,"id":"b52e51ff_024bd0b4","line":764,"updated":"2026-06-26 18:19:26.000000000","message":"Duplicate context assignment: \u0027context \u003d pecan.request.context\u0027 is assigned at line 752 and then reassigned identically at line 764. The second assignment is dead code introduced by this patch.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Harmless at runtime but introduces confusion about which assignment is authoritative, and signals copy-paste during development that could mask a future intent to use a different context.\n\n**Suggestion**:\nRemove the redundant \u0027context \u003d pecan.request.context\u0027 at line 764. The variable is already in scope from line 752.","commit_id":"3c9c9f5302a4a8b3e91caf2072c27b53a6744518"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"b7ee664dc58123e1b0e367171c089bb62a64ed6a","unresolved":false,"context_lines":[{"line_number":763,"context_line":""},{"line_number":764,"context_line":"        context \u003d pecan.request.context"},{"line_number":765,"context_line":"        strategy_obj \u003d None"},{"line_number":766,"context_line":"        if audit_template_postdata.strategy:"},{"line_number":767,"context_line":"            strategy_obj \u003d objects.Strategy.get("},{"line_number":768,"context_line":"                context, audit_template_postdata.strategy"},{"line_number":769,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":3,"id":"2c86b1ba_c38c490a","line":766,"updated":"2026-06-26 18:19:26.000000000","message":"The post() handler performs a redundant objects.Strategy.get() lookup for default_parameters validation. AuditTemplatePostType.validate() already resolved the strategy to a UUID. This second lookup is an unnecessary DB round-trip and lacks exception handling.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Eliminates an unnecessary database round-trip on every audit template creation. Wrapping the lookup in a try/except would also prevent an HTTP 500 if the strategy is deleted between validate() and post() (race condition).\n\n**Recommendation**:\nConsider either: (1) moving default_parameters validation into the existing validate() method where the strategy object is already available, or (2) wrapping this lookup in try/except to handle StrategyNotFound gracefully.","commit_id":"3c9c9f5302a4a8b3e91caf2072c27b53a6744518"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"6cac6b957dfafab79a706f09809b1f9b6cb920b5","unresolved":false,"context_lines":[{"line_number":767,"context_line":""},{"line_number":768,"context_line":"        context \u003d pecan.request.context"},{"line_number":769,"context_line":"        strategy_obj \u003d None"},{"line_number":770,"context_line":"        if audit_template_postdata.strategy:"},{"line_number":771,"context_line":"            strategy_obj \u003d objects.Strategy.get("},{"line_number":772,"context_line":"                context, audit_template_postdata.strategy"},{"line_number":773,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":4,"id":"922880c6_e73c9abb","line":770,"updated":"2026-07-01 13:43:57.000000000","message":"The POST handler re-fetches the strategy via objects.Strategy.get() (lines 770-773) even though AuditTemplatePostType.validate() already resolved the strategy to a UUID and validated goal/strategy consistency during wsme validation. This is a second lookup of the same strategy in the same request.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Removing the redundant fetch avoids an extra DB round-trip per create and makes the single source of truth for strategy resolution clearer.\n\n**Recommendation**:\nEither reuse the strategy object resolved during validate() (e.g. stash it on the request context) or add a short comment explaining why a second lookup is intentional here. Low priority; current behavior is correct.","commit_id":"589b9a1a1eaac96980cfd827ee954a9e9348eb11"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"081f5862616cad6090dccf9f2a11dcc15d7e9d4f","unresolved":false,"context_lines":[{"line_number":539,"context_line":"            raise exception.Invalid("},{"line_number":540,"context_line":"                _(\u0027default_parameters requires a strategy to be specified\u0027)"},{"line_number":541,"context_line":"            )"},{"line_number":542,"context_line":"        schema \u003d strategy.parameters_spec"},{"line_number":543,"context_line":"        if schema:"},{"line_number":544,"context_line":"            try:"},{"line_number":545,"context_line":"                # StrictDefaultValidatingDraft4Validator mutates its instance"}],"source_content_type":"text/x-python","patch_set":5,"id":"82ba1ea5_7e79f96d","line":542,"updated":"2026-07-10 20:16:35.000000000","message":"The _validate_default_parameters method skips schema validation when strategy.parameters_spec is falsy (None or empty dict). Since the base Strategy class get_schema() returns {}, strategies without defined parameters will have an empty parameters_spec. The method accepts default_parameters in th...\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: An operator can create an audit template with default_parameters for a strategy that has no parameter schema. Every subsequent attempt to create an audit from that template will fail with a confusing error message, because the inherited default_parameters trigger the \u0027no parameter spec\u0027 rejection...\n\n**Suggestion**:\nIn _validate_default_parameters, reject default_parameters when the strategy has no usable parameter schema. For example, after confirming strategy is not None, add: `if not strategy.parameters_spec: raise exception.Invalid(_(\u0027default_parameters cannot be set because the strategy has no parameter schema\u0027))`. Alternatively, treat an empty schema the same as a non-empty schema and let StrictDefaultValidatingDraft4Validator reject unexpected keys.","commit_id":"49645b83d8a1e4a9c2ff5595476740fba2a7069a"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4cb003180a9fb488e88dcead59a99ac0040ff0c7","unresolved":false,"context_lines":[{"line_number":539,"context_line":"            raise exception.Invalid("},{"line_number":540,"context_line":"                _(\u0027default_parameters requires a strategy to be specified\u0027)"},{"line_number":541,"context_line":"            )"},{"line_number":542,"context_line":"        schema \u003d strategy.parameters_spec"},{"line_number":543,"context_line":"        if schema:"},{"line_number":544,"context_line":"            try:"},{"line_number":545,"context_line":"                # StrictDefaultValidatingDraft4Validator mutates its instance"}],"source_content_type":"text/x-python","patch_set":6,"id":"c1425d60_5f82de78","line":542,"updated":"2026-07-24 17:20:09.000000000","message":"The _validate_default_parameters method skips validation entirely when strategy.parameters_spec is None or empty, allowing arbitrary default_parameters to be stored on templates whose strategies have no parameter schema. When an audit is later created from such a template, the audit creation hand...\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Operators can create audit templates with default_parameters that appear valid but cannot be used for their intended purpose. Any attempt to create an audit from such a template fails with a misleading error message that references explicitly specified parameters, when the parameters were actuall...\n\n**Suggestion**:\nIn _validate_default_parameters, when schema is falsy (None or empty), raise an error such as: raise exception.Invalid(_(\u0027default_parameters cannot be set because the strategy has no parameter schema\u0027)). Alternatively, validate that strategy.parameters_spec is non-empty before accepting default_parameters at the call sites in the post and patch handlers.","commit_id":"105d7324e4eb7b3c32c3c44355daa0d43be3f517"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"58c9c2ab4d387e7d202ae3c4c9a260ea4c59a333","unresolved":false,"context_lines":[{"line_number":539,"context_line":"            raise exception.Invalid("},{"line_number":540,"context_line":"                _(\u0027default_parameters requires a strategy to be specified\u0027)"},{"line_number":541,"context_line":"            )"},{"line_number":542,"context_line":"        schema \u003d strategy.parameters_spec"},{"line_number":543,"context_line":"        if schema:"},{"line_number":544,"context_line":"            try:"},{"line_number":545,"context_line":"                # StrictDefaultValidatingDraft4Validator mutates its instance"}],"source_content_type":"text/x-python","patch_set":6,"id":"39de52d5_9d59f5e4","line":542,"in_reply_to":"c1425d60_5f82de78","updated":"2026-07-24 17:31:19.000000000","message":"All strategies have a schema. I don\u0027t see a strategy being created without an schema,.","commit_id":"105d7324e4eb7b3c32c3c44355daa0d43be3f517"}],"watcher/db/sqlalchemy/alembic/versions/f56ba02662b4_add_default_parameters_to_audit_templates.py":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"4e6e41cfcfe9ad4d62b0c4cff446557e6050c054","unresolved":true,"context_lines":[{"line_number":18,"context_line":"def upgrade():"},{"line_number":19,"context_line":"    op.add_column("},{"line_number":20,"context_line":"        \u0027audit_templates\u0027,"},{"line_number":21,"context_line":"        sa.Column(\u0027default_parameters\u0027, sa.Text(), nullable\u003dTrue),"},{"line_number":22,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":5,"id":"e822b277_668c7251","line":21,"updated":"2026-07-20 13:58:52.000000000","message":"the type of the column does not match the one defined in the models file below","commit_id":"49645b83d8a1e4a9c2ff5595476740fba2a7069a"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"e59c4b050b19973c0991fbc612822c8d67fc5877","unresolved":false,"context_lines":[{"line_number":18,"context_line":"def upgrade():"},{"line_number":19,"context_line":"    op.add_column("},{"line_number":20,"context_line":"        \u0027audit_templates\u0027,"},{"line_number":21,"context_line":"        sa.Column(\u0027default_parameters\u0027, sa.Text(), nullable\u003dTrue),"},{"line_number":22,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":5,"id":"19780cde_676a3214","line":21,"in_reply_to":"82c65e01_861f9416","updated":"2026-07-24 17:13:48.000000000","message":"So in the end it doesn\u0027t make difference for the alembic migration, since it is going to create as Text type. The JSONEncodedDict adds additional methods only for the ORM. \nI updated the patch so it follows the other migrations and make easier to check if the type is aligned with the models.","commit_id":"49645b83d8a1e4a9c2ff5595476740fba2a7069a"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"99383a785b3ba6bc0cf0cb3332f928c7ea82b2e8","unresolved":true,"context_lines":[{"line_number":18,"context_line":"def upgrade():"},{"line_number":19,"context_line":"    op.add_column("},{"line_number":20,"context_line":"        \u0027audit_templates\u0027,"},{"line_number":21,"context_line":"        sa.Column(\u0027default_parameters\u0027, sa.Text(), nullable\u003dTrue),"},{"line_number":22,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":5,"id":"82c65e01_861f9416","line":21,"in_reply_to":"e822b277_668c7251","updated":"2026-07-21 11:27:05.000000000","message":"hum, right, other columns are using models.JSONEncodedList() type, which translate to a Text but adds the json enconde/decode. Going to update that, thanks!","commit_id":"49645b83d8a1e4a9c2ff5595476740fba2a7069a"}],"watcher/tests/unit/api/v1/test_audits.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"e3b1f4174949fe959c4b81f08ac3b1052528590d","unresolved":true,"context_lines":[{"line_number":1261,"context_line":"            self.context, response.json[\u0027uuid\u0027]"},{"line_number":1262,"context_line":"        )"},{"line_number":1263,"context_line":"        self.assertEqual(override_params, new_audit.parameters)"},{"line_number":1264,"context_line":""},{"line_number":1265,"context_line":"    @mock.patch.object(deapi.DecisionEngineAPI, \u0027trigger_audit\u0027)"},{"line_number":1266,"context_line":"    @mock.patch(\u0027oslo_utils.timeutils.utcnow\u0027)"},{"line_number":1267,"context_line":"    def test_create_audit_with_name(self, mock_utcnow, mock_trigger_audit):"}],"source_content_type":"text/x-python","patch_set":2,"id":"f64f3bd6_25195717","line":1264,"updated":"2026-06-24 13:15:21.000000000","message":"I miss a test to validate param dict merging from default_params in audittemplates with strategy default and/or audit parameters for a strategy witn multiple parameters. One parameter may come from the AT, other from Audit params and other from audit default.","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"893e4bffd71a0567e174b3712568a435ab6653f9","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"            self.context, response.json[\u0027uuid\u0027]"},{"line_number":1262,"context_line":"        )"},{"line_number":1263,"context_line":"        self.assertEqual(override_params, new_audit.parameters)"},{"line_number":1264,"context_line":""},{"line_number":1265,"context_line":"    @mock.patch.object(deapi.DecisionEngineAPI, \u0027trigger_audit\u0027)"},{"line_number":1266,"context_line":"    @mock.patch(\u0027oslo_utils.timeutils.utcnow\u0027)"},{"line_number":1267,"context_line":"    def test_create_audit_with_name(self, mock_utcnow, mock_trigger_audit):"}],"source_content_type":"text/x-python","patch_set":2,"id":"de1add1c_b1e5cded","line":1264,"in_reply_to":"f64f3bd6_25195717","updated":"2026-07-01 10:02:02.000000000","message":"Done","commit_id":"074a4ce26e174baa9cce9b2ca9a9d5111e6c70d0"}]}
