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

airflow_log_cleanup_workflow module docs are invalid markdown #4750

Closed
sarayourfriend opened this issue Aug 13, 2024 · 2 comments · Fixed by #4770
Closed

airflow_log_cleanup_workflow module docs are invalid markdown #4750

sarayourfriend opened this issue Aug 13, 2024 · 2 comments · Fixed by #4770
Assignees
Labels
📄 aspect: text Concerns the textual material in the repository 🛠 goal: fix Bug fix 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: documentation Related to Sphinx documentation

Comments

@sarayourfriend
Copy link
Contributor

sarayourfriend commented Aug 13, 2024

Description

The following two lines produce invalid markdown due to the angle brackets used to surround the type annotations:

- maxLogAgeInDays:<INT> - Optional
- enableDelete:<BOOLEAN> - Optional

This causes an issue for e.g., prettier parsing the DAG docs file:

documentation/_serve/catalog/reference/DAGs.html
[error] documentation/_serve/catalog/reference/DAGs.html: SyntaxError: Unexpected closing tag "p". It may happen when the tag has already been closed by another tag. For more info see https://www.w3.org/TR/html5/syntax.html#closing-elements-that-have-implied-end-tags (1292:40)
[error]   1290 | <p><code class="docutils literal notranslate"><span class="pre">--conf</span></code> options:</p>
[error]   1291 | <ul class="simple">
[error] > 1292 | <li><p>maxLogAgeInDays:<INT> - Optional</p></li>
[error]        |                                        ^^^^
[error]   1293 | <li><p>enableDelete:<BOOLEAN> - Optional</p></li>
[error]   1294 | </ul>
[error]   1295 | </section>

Changing these to use some other syntax to annotate the type would solve this. The angle bracket syntax isn't used anywhere else, and the types do not correspond to JSON types, which is the input format, nor exactly match Python type names, so I'm not sure if this is following some specific but unfamiliar to me syntax for annotation.

Fix

We can fix this by changing these to use parenthesis.

@sarayourfriend sarayourfriend added 🟩 priority: low Low priority and doesn't need to be rushed 🛠 goal: fix Bug fix 📄 aspect: text Concerns the textual material in the repository 🧱 stack: documentation Related to Sphinx documentation labels Aug 13, 2024
@AetherUnbound
Copy link
Contributor

AetherUnbound commented Aug 15, 2024

The file referenced looks like it's the produced HTML for our documentation site, which is in documentation/_serve 🤔 I'm not sure that's something prettier should be running on in most cases (particularly ov just lint)...either way, easy enough fix.

In order to replicate this, you can run:

  1. ov just documentation/live
  2. ov pnpm exec prettier documentation/_serve/catalog/reference/DAGs.html

@sarayourfriend
Copy link
Contributor Author

sarayourfriend commented Aug 19, 2024

The file referenced looks like it's the produced HTML for our documentation site, which is in documentation/_serve

Oh, true, I hadn't noticed that. Probably worth an issue for that one! It might have been the way I was running prettier at the time, though, I haven't seen this error come up when running it again.

On the other hand, we probably should validate the HTML output, if this kind of thing is possible, provided we want the HTML output to be valid 😅

I'm guessing if we did need angle brackets we'd actually need to transform them (at which step I'm not sure) into HTML escape sequences, like you would when wanting to render angle brackets in HTML anyway (including in Markdown, because it interprets angle brackets as HTML anyway... assuming the contents aren't a URL?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📄 aspect: text Concerns the textual material in the repository 🛠 goal: fix Bug fix 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: documentation Related to Sphinx documentation
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants