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

Display error with API Documentation for the AWS Lambda Function Resource #2624

Closed
Tracked by #3780 ...
toriancrane opened this issue Jul 18, 2023 · 3 comments
Closed
Tracked by #3780 ...
Assignees
Labels
area/api-docs area/providers kind/enhancement Improvements or new features resolution/fixed This issue was fixed size/L Estimated effort to complete (up to 10 days).
Milestone

Comments

@toriancrane
Copy link

When accessing the AWS Lambda Function resource page, there is a section called "CloudWatch Logging and Permissions" near the top of the page that I believe should be nested under the "Example Usage" section.

It is the first item on the right hand navigation menu.
image

It also seems to be missing the the language selector/tabbing functionality.

image

@toriancrane toriancrane changed the title Error with API Documentation for the AWS Lambda Function Resource Display error with API Documentation for the AWS Lambda Function Resource Jul 18, 2023
@thomas11
Copy link
Contributor

Thank you for reporting this, @toriancrane. I agree it doesn't look quite right. I've asked some folks internally and will report back.

@thomas11
Copy link
Contributor

Likely part of pulumi/pulumi-terraform-bridge#1291. The examples are not detected correctly, which is also why the language chooser is missing.

@guineveresaenger
Copy link
Contributor

guineveresaenger commented Feb 9, 2024

Summary of Internal Discussion:

  • we should likely take a more straightforward approach and take upstream docs in the order they appear, providing language choosers for every code snippet. Joe also had some comments in this issue.
  • this means streamlining some of the "Example Usage" logic that is scattered across the bridge and the docsgen templates.

Findings so far:

Our assumptions for upstream docs are too specialized and do not match reality.

  • we assume code snippets only exist in an "Example Usage" section
    • in fact we hard code an expectation that an "Example Usage" section exists with that name which is potentially brittle
    • additionally if a subsection of "Example Usage" does not have code in it, we get odd behavior
  • we assume if there are more examples that they are prefixed with an H3 markdown header
  • we base ordering, docsgen, metrics, and display logic on the above assumption

I spent some time hunting down how an "examples section" is achieved in the Terraform Bridge). The hope was that by expanding the {{%% examples %%}} shortcode in the bridge, either by wrapping all of the resource's docs in the shortcode, or wrapping each example individually, to include H2 headings we could achieve code wrappers for everything. Unfortunately, this is not possible, as docsgen creates a <pulumi-examples> div using duplicate logic based on the presence of a ## Example Usage header. Additionally, it turns out we have code snippets in the Supporting Types descriptions as well which behave much like the CloudWatch snippet in Lambda and should be included in a fix as well.

The bridge code that translates examples is the same code finds and translates all code snippets, but only marks H3 sections as an "example" via {{%% examples %%}} and {{%% example %%}} short code, resulting in the missing chooser. This also incidentally colors the translation metrics, as code that isn't in any "Example Usage" section counts towards example conversion metrics.

Next, I'll look into the logic surrounding this template - due to some duplicate logic we'll have to clean this up in parallel with the bridge.

The templating logic seems also responsible for some of the mismatching of example code to titles, and odd ordering of headers (the bridge order looks consistent across resources) when upstream does not match our expectations.

Unfortunately, it also appears that there is registry code that relies on an examples section so we may need some cleanup.

Suggestions

Long-term
The ultimate goal here, I think, should be to remove the special templating behavior for "Example Usage" and re-write templates and logic to wrap all code found in an individual language chooser. This would result in a display that closely follows upstream presentation and order. This would be a change noticeable mostly in our biggest providers (aws, gcp, kubernetes) as many smaller provider don't have nested examples in this way.

  1. Remove/rewrite Example Usage templating behavior; implement a smaller, flexible wrapper template for all code snippets, keep upstream ordering. Possible to keep the div but base it purely on header order.
  2. Clean up bridge code to remove shortcodes and simplify the nested docs logic by flattening each H2 and H3 section
  3. Clean up registry code that relies on having an Examples section if necessary.

Immediate
For this particular issue on the lambda docs page, we have a potential quick fix: we can patch the upstream docs so that the "CloudWatch Logging" section can be interpreted as part of the Examples section. We don't know if having this section be an example is the original upstream intent, so this fix is not ideal. But since this is such a frequently-visited page, I would recommend we do this today, while leaving this issue open and continue to implement the more long-term and general fix.

guineveresaenger added a commit that referenced this issue Feb 10, 2024
…ge section (#3410)

Partial and temporary hack for
#2624.
Adds a patch that rewrites the "CloudWatch Logging and Permission"
section as a nested H3 example, which gets rid of the inline code
display at the top of out [Lambda Function landing
page](https://www.pulumi.com/registry/packages/aws/api-docs/lambda/function/).

Note:
Elides display the incorrect "Lambda retries" section. A full fix for
#2624 will address this and bring it back as the [description-only
section that it
is](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_function#lambda-retries).
This is a temporary hack and should be fully addressed via #2624, after
which this patch should be removed.

The unrelated patch churn seems legitimate:
 - the deleted patch was included in an earlier patch 0036
- the changed function title in 0022 is the patch anchor and does not
affect our patch content.
---

Here's a screenshot of what the page should look like in the registry
(please excuse the tiny image, but you can see the improvement)
<img width="863" alt="Screenshot 2024-02-09 at 11 14 59 AM"
src="https://github.com/pulumi/pulumi-aws/assets/13116240/8395c58e-f3e6-4779-add1-567a39cc0a9e">
@sean1588 sean1588 modified the milestones: 0.100, 0.101 Feb 21, 2024
@sean1588 sean1588 added the size/L Estimated effort to complete (up to 10 days). label Feb 21, 2024
github-merge-queue bot pushed a commit to pulumi/pulumi that referenced this issue Feb 23, 2024
This pull request depends on corresponding changes in the
pulumi-terraform bridge.

The bridge will mark up each code section it finds (not just ones found
under an "Example Usage" header) with an HTML comment - currently this
will be `<!--Begin Code Chooser -->` and `<!--End Code Chooser -->` but
the name is subject to change.

In this PR, we are adding logic to detect if an incoming Description
block has an `{{% examples }}` shortcode markup: if yes, we will process
it as before. If no, we will switch to new functionality that relies on
documentation with code sections having no short codes, no "Examples
Section" logic, and will generate every code section it finds in an
incoming schema that is in the Description/Content section of a registry
page with a chooser and language choosables.

This work is part of pulumi/pulumi-aws#2624.


Edit: unit tests are added

This PR is a prerequisite for
pulumi/pulumi-terraform-bridge#1689. It aims to
support both legacy and new behavior.



<!--- 
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->

Fixes # (issue)

## Checklist

- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
  - [x] I have formatted my code using `gofumpt`

<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] I have added tests that prove my fix is effective or that my
feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [ ] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
iwahbe added a commit to pulumi/pulumi-terraform-bridge that referenced this issue Mar 7, 2024
…1689)

**IMPORTANT** This cannot be merged until the bridge has updated
pulumi/pulumi to a version that includes [these
changes](pulumi/pulumi@26bdac7).

After that, merging will be safe.


**Note**: pulumi/pulumi#15475 needs to be merged
first for these changes to be safe.

This PR aims to remove the nested "Example Usage" logic from the bridge,
[as discussed here](pulumi/registry#3498).

The issue this is aimed at fixing is
pulumi/pulumi-aws#2624 but there should be a
myriad of other issues that will be addressed by this change.

In a nutshell, this PR removes any `{{% example(s) }}` injection from
the `Description` fields of a resource and instead detects any and all
code conversions from TF/HCL --> pulumi languages, and surrounds them
with an HTML comment.
This is currently 
```
const (
	startPulumiCodeChooser = "<!--Start PulumiCodeChooser -->"
	endPulumiCodeChooser   = "<!--End PulumiCodeChooser -->"
)
```

Marking up any converted code blocks using the new format will serve
_only_ as a marker for registry docsgen to surround this converted code
with a language chooser.

I refrained from going full Markdown parser because I believe it would
be an even larger rewrite. Instead, we detect code fences in the
docstring input, and append text and converted code section to the
output, with the following caveats:

- If a code block is part of a Section (i.e. has a Markdown header), if
we encounter errors in code conversion, the new functionality strips the
entire section automatically. Non-headered code blocks with such an
error will also continue to be stripped.
- Using indices rather than line splitting allows us to get rid of the
awkward `<break>` placeholders for the Import section.
- The Import section no longer needs special handling in
`convertExamplesInner`: it is valid code and will be retained as such
- With a little bit of extra parsing of the code fence syntax
highlighter, we can capture a few more cases of perfectly valid json or
yaml that we have elided previously as "failure to convert from TF".
- With the removal of examples, titles (which never actually made it
into the schema) are now obsolete. We will refer to resource paths when
logging conversion failures. We have done so all along when there was no
`exampleTitle`, and this change affects only the wording of logging
output.
- The `isFrontMatter` variable was removed as well. To the best of my
knowledge, Hugo front matter does not get piped through the bridge, and
is generated by the registry instead.
- Handwritten docs continue to bypass `convertExamplesInner`, with an
added caveat that both third party and our own docs may add the old
example shortcodes. This change will allow us to [remove some of
these](https://github.com/pulumi/pulumi-aws/blob/master/docs/resource/aws_acm_certificate_validation.examples.md).
Registry docsgen will continue to support the shortcode way of doing
things, since native providers conform to that expectation separately.

---------

Co-authored-by: Ian Wahbe <[email protected]>
@sean1588 sean1588 modified the milestones: 0.101, 0.102 Mar 8, 2024
@sean1588 sean1588 added the resolution/fixed This issue was fixed label Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api-docs area/providers kind/enhancement Improvements or new features resolution/fixed This issue was fixed size/L Estimated effort to complete (up to 10 days).
Projects
None yet
Development

No branches or pull requests

4 participants