search

operate-first / operate-first.github.io-old

GNU General Public License v3.0
18 stars 42 forks source link

re-build of the website fails #267

Closed oindrillac closed 3 years ago

oindrillac commented 3 years ago

Our re-build triggers[1][2] of the website failed

Log:

"@rafaelquintanilha/gatsby-transformer-ipynb" threw an error while running the onCreateNode lifecycle:

hr is a void element tag and must neither have `children` nor use `dangerouslySetInnerHTML`.
    in hr
    in remarkElement
    in Fragment
    in Root
    in ReactMarkdownWithHtml
    in div
    in Context.Consumer
    in Context.Consumer
    in StyledComponent
    in styled.div
    in div
    in Context.Consumer
    in Context.Consumer
    in StyledComponent
    in Cell
    in div
    in Context.Consumer
    in Context.Consumer
    in StyledComponent
    in Cells
    in div
    in NotebookRender

To Reproduce:

[1] https://www.travis-ci.com/github/operate-first/operate-first.github.io/builds/232293009 [2] https://www.travis-ci.com/github/operate-first/operate-first.github.io/builds/232293093

oindrillac commented 3 years ago

@harshad16 and @Gkrumbach07 looked into this issue and this is what was detected:

tldr: long line breaks in notebooks such as ------ create hr tags in notebooks which causes the notebook renderer to error out due to this line of code which sets escapeHtml to false essentially trusting that the html doesn't contain tags like that which is by default true to skip the "dangerous html".

Current Fix: To remove long line breaks for notebooks within repos which are linked to the site.

Expected Long Term Fix: Add a plugin that detects bad HTML.

Hey there 👋

This is a Slack discussion you wanted me to capture.

Oindrilla Chatterjee wrote: Any React/Gatsby experts in the house who can help figure out why the website build is failing? https://github.com/operate-first/operate-first.github.io/issues/267

Francesco Murdaca wrote: Gage Krumbach?

Harshad wrote: we will also take a look :+

Harshad wrote: Oindrilla Chatterjee what is the last know commit you could relloct which build successfully earlier

Parth Soni wrote: Harshad - I think this one was last successful commit - https://github.com/operate-first/operate-first.github.io/commit/78148befc9f9bc7120bee32dd90d8f5228a2c63d

Oindrilla Chatterjee wrote: yes, that is the one.

Harshad wrote: i tried to find the solution, unfortunately couldnt solve it on friday.

Oindrilla Chatterjee wrote: Harshad thanks for trying! any idea on what could be broken ?

Harshad wrote: the package gatsby-transformer-ipynb is not been updated for a while

Harshad wrote: the error hr is a void element tag and must neither have `children` nor use `dangerouslySetInnerHTML`. occurs due to an hr tag been opend like <hr> where as it should be <hr /> we need to iupdate the subsequent module which creates this had a hard time finding it

Harshad wrote: this is my hunch , can be worng as well

Oindrilla Chatterjee wrote: right, that seems probable. I think many of our npm packages are not up to date.

Gage Krumbach wrote: Oindrilla Chatterjee I was able to get it to run without the error. I think it has to do with some markdown issue. Currently I dont know exactly what fixed it but i will take a further look tomorrow.

Harshad wrote: hey gage , you also provide the pr link here. i would like to learn about the issue and fix

Gage Krumbach wrote: In dont have a pr at the moment but can give a thought of what i think is happening.

There is a jupyter notebook that operate first is trying to render which contains a markdown cell. That markdown cell has html in it that happens to also have a hr tag as well. This errors out because of this line of code. (https://github.com/rafaelquintanilha/notebook-render/blob/master/src/index.tsx#L230). escapeHtml is by default true which skips the "dangerous html" but the rafaelquintanilha/notebook-render package sets it to false. essentially trusting that the html doesn't contain tags like that.

I dont know which jupyter notebook caused it. And somehow the fix I had no longer works so ill keep trying until it does again and will let this thread know.

Oindrilla Chatterjee wrote: That seems like good progress! thanks for looking into this Gage Krumbach and Harshad

Oindrilla Chatterjee wrote: I tried updating the packages and it seems to not throw and error anymore. Here's the code https://github.com/oindrillac/operate-first.github.io.git. Can either of you please verify whether it works for you? Harshad Gage Krumbach

Harshad wrote: i still see the issue Oindrilla Chatterjee, i think we need to investigate on what gage proposed, it definitely is a issue in rendering, either we have to fix up the notebook or update the package upstream for handling of such issue, in my opinion.

Gage Krumbach wrote: this is the notebook that is causing an issue i think https://github.com/aicoe-aiops/ocp-ci-analysis/blob/master/notebooks/data-sources/oc-github-repo/github_PR_EDA.ipynb

Gage Krumbach wrote: it also could be a more grand issue if that notebook was working prior.

Oindrilla Chatterjee wrote: Gage Krumbach I have worked on that notebook, and the code dividers were only added recently. So those could be removed

Oindrilla Chatterjee wrote: I can add a PR to fix it upstream if that helps

Gage Krumbach wrote: it could. I dont know exactly what about the notebook is causing an issue. because there is no deliberate hr tag in there so it must be generated somewhere.

Oindrilla Chatterjee wrote: I think it would be coming from the markdown dividers ------------ after cell [11]

Oindrilla Chatterjee wrote: I modified the notebook and pushed it to my fork. Can we adding this URL to the gatsby-config.js https://github.com/oindrillac/ocp-ci-analysis.git to replace the https://github.com/aicoe-aiops/ocp-ci-analysis.git to see if it builds succesfully?

Oindrilla Chatterjee wrote: if it works, I can add a PR upstream to fix it

Gage Krumbach wrote: ill give it a run

Gage Krumbach wrote: Oindrilla Chatterjee It looks like that worked! Im still not sure why those have html inside them. maybe it could be due to the ----------------------------------- having two many dashes. Because it only take 3 dashes --- to make a break, so im guessing there were extra dashes as children in the hr tag. its only a hunch and probably not right but at least it works now

Gage Krumbach wrote: in the long run there should probably be some plugin that detects bad HTML. I saw that the newer version of the notebook renderer recommended to use one. Especially if the notebooks are being pulled from external sources

_Transcript of Slack thread: https://operatefirst.slack.com/archives/C01RMPVUUK1/p1625843112366200?thread_ts=1625843112.366200&cid=C01RMPVUUK1_

oindrillac commented 3 years ago

/assign @oindrillac /assign @harshad16 /assign @Gkrumbach07

sesheta commented 3 years ago

@oindrillac: GitHub didn't allow me to assign the following users: Gkrumbach07.

Note that only operate-first members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide

In response to [this](https://github.com/operate-first/operate-first.github.io/issues/267#issuecomment-879482200): >/assign @oindrillac >/assign @harshad16 >/assign @Gkrumbach07 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.