Skip to content
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 for docs generation when no items key for a schema property of array type is present. #1052

Conversation

mrinaudo-aws
Copy link
Contributor

Issue #, if available:

Description of changes:
Fix for documentation generation when no items key for a schema property of array type is present. When running cfn generate, for example against a schema of a CloudFormation hook that is missing the items node for a given property of array type, while the schema validation succeeds, the document generation logic fails with a KeyError because it assumes the items node is present in the input data:

[...]
    if "$ref" in prop["items"]:
                 ~~~~^^^^^^^^^
KeyError: 'items'

This change fixes this behavior by leaving the schema validation unchanged, and by defaulting to an empty map for items if items itself is not present in the input data. Proposed changes also include a warning message for the user to take action if needed.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mrinaudo-aws mrinaudo-aws requested a review from kddejong January 4, 2024 17:14
@@ -959,6 +959,22 @@ def __set_property_type(prop_type, single_type=True):
lambda markdown_value: f"List of {markdown_value}"
) # noqa: E731

if "items" not in prop.keys():
LOG.warning(
'Warning: found schema property of array type with no "items" key; \n'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the best default?

"items": {
  "type": "string"
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not the default value here, it is just an example in the event the (hook's) author chooses to have an input property of array type, whereas its items are of the string desired type.

@brianlaoaws
Copy link

Do you mind posting what the docs look like after this change?

@mrinaudo-aws
Copy link
Contributor Author

Do you mind posting what the docs look like after this change?

@brianlaoaws sure - here is an example output whereas in the schema there were no items defined for a property, defaulting to _Type_: List of Map:

# (sample value omitted)

## Activation

To activate a hook in your account, use the following JSON as the `Configuration` request parameter for [`SetTypeConfiguration`](https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_SetTypeConfiguration.html) API request.

### Configuration

<pre>
{
    "CloudFormationConfiguration": {
        "<a href="https://docs.aws.amazon.com/cloudformation-cli/latest/userguide/hooks-structure.html#hooks-hook-configuration" title="HookConfiguration">HookConfiguration</a>": {
            "<a href="https://docs.aws.amazon.com/cloudformation-cli/latest/userguide/hooks-structure.html#hooks-targetstacks" title="TargetStacks">TargetStacks</a>":  <a href="#footnote-1">"ALL" | "NONE"</a>,
            "<a href="https://docs.aws.amazon.com/cloudformation-cli/latest/userguide/hooks-structure.html#hooks-failuremode" title="FailureMode">FailureMode</a>": <a href="#footnote-1">"FAIL" | "WARN"</a> ,
            "<a href="https://docs.aws.amazon.com/cloudformation-cli/latest/userguide/hooks-structure.html#hooks-properties" title="Properties">Properties</a>" : {
                "<a href="(sample value omitted)" title="(sample value omitted)">(sample value omitted)</a>" : <i>[ Map, ... ]</i>
            }
        }
    }
}
</pre>

## Properties

#### (sample value omitted)

(sample value omitted)

_Required_: No

_Type_: List of Map


---

## Targets

* `(sample value omitted)`

---

<p id="footnote-1"><i> Please note that the enum values for <a href="https://docs.aws.amazon.com/cloudformation-cli/latest/userguide/hooks-structure.html#hooks-targetstacks" title="TargetStacks">
TargetStacks</a> and <a href="https://docs.aws.amazon.com/cloudformation-cli/latest/userguide/hooks-structure.html#hooks-failuremode" title="FailureMode">FailureMode</a>
might go out of date, please refer to their official documentation page for up-to-date values. </i></p>

@mrinaudo-aws mrinaudo-aws requested a review from ammokhov January 4, 2024 19:41
@ammokhov ammokhov merged commit f0e3769 into aws-cloudformation:master Jan 4, 2024
12 checks passed
@mrinaudo-aws mrinaudo-aws deleted the fix-documentation-generation-prop-items branch January 4, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants