-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix genconfig does not fail when schema validation failed #425
Conversation
iotedgedev/deploymentmanifest.py
Outdated
@@ -190,6 +190,9 @@ def _validate_json_schema(self, schema_object, json_object, schema_type): | |||
self.output.warning("%s schema validation failed. Please see previous logs for more details" % schema_type) | |||
else: | |||
self.output.info("%s schema validation passed." % schema_type) | |||
|
|||
if error_detected: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line 189 has if error_detected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
besides it already has error_detected, do we still need validation_success?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the error_detected and reuse validation_success. We need to declare validation_success outside of try to handle situations with exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed error_detected? we have two blocks of error_detected (189 and 194)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the commit that removes error_detected, I removed both of them and use validation_success instead: 60cac66
@@ -146,7 +146,7 @@ def validate_deployment_template(self): | |||
validation_success = True | |||
try: | |||
template_schema = json.loads(urlopen(Constants.deployment_template_schema_url).read().decode()) | |||
self._validate_json_schema(template_schema, self.json, "Deployment template") | |||
validation_success = self._validate_json_schema(template_schema, self.json, "Deployment template") | |||
except Exception as ex: # Ignore other non shcema validation errors | |||
self.output.info("Unexpected error during deployment template schema validation, skip schema validation. Error:%s" % ex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exception happens and the validation is still success?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only schema failures should be treated as validation failure. Other exceptions such as network error should not fail the validation. iotedgedev will only print related error and skip validation.
Some Azure CLI modules breaks the automation test, I will investigate and see whether I can provide a quick fix |
Fix genconfig does not fail when schema validation failed #424
AB#1696447