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

Switch Markdown engine to markdown-it-py #19702

Merged
merged 34 commits into from
Jun 21, 2022
Merged

Conversation

torbjornvatn
Copy link
Contributor

As mentioned in #16435 will switching to markdown-it-py give much more advanced Markdown rendering and make it behave almost like to Github Flavored Markdown

closes: #16435
related: #19644

@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Nov 19, 2021
@torbjornvatn
Copy link
Contributor Author

@ferruzzi This one passed the Static checks

tests/www/test_utils.py Outdated Show resolved Hide resolved
@torbjornvatn
Copy link
Contributor Author

@ashb I would ideally find a way to remove the display: block; property from a <summary> inside a <display> to get a ‣ in front of the collapsible content just as here on Github. But I couldn't figure out how to change any of the CSS really 🤷🏼

Example:

Click to expand!

Heading

  1. A numbered
  2. list
    • With some
    • Sub bullets

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Jan 16, 2022
@uranusjr
Copy link
Member

Any updates on this?

@github-actions github-actions bot removed the stale Stale PRs per the .github/workflows/stale.yml policy file label Jan 17, 2022
@torbjornvatn
Copy link
Contributor Author

torbjornvatn commented Jan 17, 2022

Seems like the conflict in airflow/www/utils.py is caused by someone else attempting to do something similar, but with a different Markdown lib than me. Will have to review that change to see if it's merged into master and makes this PR obsolete

@torbjornvatn
Copy link
Contributor Author

It was something else actually, fixed the conflict now

@uranusjr
Copy link
Member

So do we need to do anything special to <summary>’s display: block?

@torbjornvatn
Copy link
Contributor Author

So do we need to do anything special to <summary>’s display: block?

Are you asking me @uranusjr? I don't really know, do you want me to test it somehow?

@uranusjr
Copy link
Member

You said above

I would ideally find a way to remove the display: block; property from a <summary> inside a <display> to get a ‣ in front of the collapsible content just as here on Github.

I was just following that up.

@ashb
Copy link
Member

ashb commented Jan 22, 2022

@bbovenzi CSS questions - can you help out please

@torbjornvatn
Copy link
Contributor Author

You said above

I would ideally find a way to remove the display: block; property from a <summary> inside a <display> to get a ‣ in front of the collapsible content just as here on Github.

I was just following that up.

🤦🏼‍♂️ yes of course. Sorry for not remembering my own questions.

A CSS fix for this would be highly appreciated @bbovenzi The working ‣ for expanding the section is kind of important for the UX

@torbjornvatn
Copy link
Contributor Author

A friendly reminder on this one @bbovenzi

I can do the CSS changes my self, I just needs some pointers on how

@bbovenzi
Copy link
Contributor

bbovenzi commented Mar 8, 2022

A friendly reminder on this one @bbovenzi

I can do the CSS changes my self, I just needs some pointers on how

My apologies. I kept missing this PR.

We should be able to manipulate the css for markdown simply by adding something like

.rich_doc summary {
  display: block;
}

to main.css

@uranusjr
Copy link
Member

The docker-compose quick start is failing because it misses the markdown-it-py package. Maybe there’s some dependency list somewhere we need to fix?

Not sure what’s going on with the Helm Chart tests.

@potiuk
Copy link
Member

potiuk commented May 12, 2022

Docker compose quick-start tests test starting airflow via docker compose and check if the

There is nothing "special" about them they are the usual "integration tests" run by pytest using breeze environment (mainly to get the right dependencies):

It's rather easy to follow what they do:

Started by breeze docker-compose-tests

run: breeze docker-compose-tests

Here are the tests (standard pytest-run python code):

https://github.com/apache/airflow/blob/main/docker_tests/test_docker_compose_quick_start.py

They are also super easy to reproduce and run - you just need to follow what is done in the ci.

There is no magic there. If they do not start, it means that something does not work and you cannot inspect airflow after you use production image to ru docker-compose with airflow.

@potiuk
Copy link
Member

potiuk commented May 12, 2022

Those tests are converted to run standard "breeze" commands and you can always easily locally reproduce it using breeze.

@potiuk
Copy link
Member

potiuk commented May 12, 2022

I just recommend to try to reproduce it following it step-by-step.

@torbjornvatn
Copy link
Contributor Author

@potiuk and @uranusjr I'm having a really hard time getting all these checks to be ✅ at the same time

The last one was something with the python cache or something, and I'm not allowed to rerun individual workflows either as far as I can see.

Screenshot 2022-06-21 at 09 05 12

@potiuk
Copy link
Member

potiuk commented Jun 21, 2022

Intermittent problem. I just re-run failed jobs only.

BTW. There is the "re-run failed jobs" button at the top (not sure if you can do it as non-committer though).

@torbjornvatn
Copy link
Contributor Author

🎉 @potiuk
It worked

@potiuk potiuk merged commit 88363b5 into apache:main Jun 21, 2022
@potiuk
Copy link
Member

potiuk commented Jun 21, 2022

Woooohooo 🎉 🎉 🎉 🎉 🎉 🎉

potiuk pushed a commit to potiuk/airflow that referenced this pull request Jun 29, 2022
@ephraimbuddy ephraimbuddy added this to the Airflow 2.3.3 milestone Jun 30, 2022
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Jun 30, 2022
details summary {
display: list-item;
}

.menu-scroll {
max-height: 300px;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you should provide and test PR with a fix @tronlightracer ? This is the best way to become one of > 2100 contributors to Airflow (mostly people like you).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:webserver Webserver related Issues full tests needed We need to run full set of tests for this PR to merge type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch Markdown engine to markdown-it-py
6 participants