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 : System Message: ERROR/3 (<string>, line 18) Unknown directive t… #632

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

novicejava1
Copy link

Sorry previous pull request got closed accidentallty while doing sync branch.

"This pull request is a fix to update the code to utilize sphinx for building the html documents instead of docutils which currently does not support the directive type "toctree" due to which there is a system error below the "Table of Contents" section."

datagrepper/app.py Outdated Show resolved Hide resolved
datagrepper/app.py Outdated Show resolved Hide resolved
@novicejava1
Copy link
Author

Fixed as per recommendations and removed unnecssary code.

@novicejava1
Copy link
Author

novicejava1 commented Jun 29, 2023

HI @abompard , i was able to fix the precommit Checks on the files. But the test_index until test is failing at the assertion due to following special characters introducted in the generated html docs. But when i launch the app and view the source code from browser i dont see those special characters instead it shows the "apostrophe" correctly. Any suggestion

<h1>Datagrepper\xe2\x80\x99s documentation<a class="headerlink" href="#datagrepper-s-documentation" title="Permalink to this headline">\xc2\xb6</a></h1>

@novicejava1
Copy link
Author

HI @abompard , fixed the issue with test_api and index page assertion looking fine now. Also precommit checks are fine. Please have a look

@abompard
Copy link
Member

Hey @novicejava1 , could you rebase the PR please? Thanks

@novicejava1
Copy link
Author

Hi @abompard ,

After cloning the repo https://github.com/novicejava1/datagrepper.git. I could see the following from which i tried to rebase by picking up the first commit and did a re-push.

pick 392882d Run CI on Fedora
f 5ffb7a5 Don't use tox-poetry, it's deprecated
f 6c97f67 Support python 3.10+
f bb50c15 Use the shared liccheck config
f ac3a812 Fix pre-commit run in CI
f e600f86 Add dep in CI
f 03e374f Update pre-commit hook asottile/pyupgrade to v3.8.0
f 9346feb Update pre-commit hook asottile/pyupgrade to v3.9.0
f ffaddc8 Update pre-commit hook psf/black to v23.7.0
f 19f5516 Fix : System Message: ERROR/3 (, line 18) Unknown directive type toctree. #537

@novicejava1 novicejava1 force-pushed the toctreefix branch 4 times, most recently from b7829de to 6ad6ad7 Compare August 22, 2023 17:43
@abompard
Copy link
Member

Github still says there's some conflict on poetry.lock. Do you need help resolving that? This should work:

git fetch origin
git rebase origin/develop

You'll need to resolve a conflict in poetry.lock, I suggest you do:

rm poetry.lock
poetry lock --no-update
git add poetry.lock
git rebase --continue

@novicejava1
Copy link
Author

Hi @abompard , please have a look now. I tried to resolve the conflicts

@abompard
Copy link
Member

Hi! Github is still reporting conflicts. Feel free to ask if you need help with git.

@novicejava1
Copy link
Author

Hi @abompard ,

I am unable to see any conflicts on my side. This is the message i see on my pull request.

This branch has no conflicts with the base branch
Only those with write access to this repository can merge pull requests.

@abompard
Copy link
Member

Huh, Github tells me "This branch cannot be rebased due to conflicts". I'll try to rebase it and push.

…ype toctree.

Fixes: fedora-infra#537

Signed-off-by: Aurélien Bompard <aurelien@bompard.org>
@abompard
Copy link
Member

Here are a few issues that I think must be improved:

  • the sphinx docs are currently rebuilt entirely on each page request. Ideally sphinx would only be called once and the html files would be served on request
  • the |URL| replacement pattern is actually static, it should depend on where datagrepper is installed (as it used to)
  • the sphinx theme still contains elements that come with the alabaster theme (see the bottom of the pages)

Thanks for your work on this!

@abompard
Copy link
Member

I've pushed a change with a simplisitic fix to this issue, but I think your idea of using sphinx instead of docutils has merits and should be explored, if you're still interested.

@novicejava1
Copy link
Author

Thanks @abompard for your inputs. I think i need to review my code further.

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.

None yet

2 participants