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

Docs for daemons #346

Merged
merged 1 commit into from
Dec 5, 2024
Merged

Docs for daemons #346

merged 1 commit into from
Dec 5, 2024

Conversation

labkode
Copy link
Member

@labkode labkode commented Jul 15, 2024

Fixes #296

@labkode labkode changed the title draft docs for daemons Docs for daemons Jul 18, 2024
@labkode labkode marked this pull request as ready for review July 18, 2024 09:16
@labkode labkode force-pushed the daemons branch 3 times, most recently from 300d24b to 910beb0 Compare July 18, 2024 09:27
@labkode labkode requested a review from bari12 July 18, 2024 09:28
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
@labkode labkode force-pushed the daemons branch 2 times, most recently from 827df06 to 2810abd Compare July 19, 2024 08:34
Copy link
Member Author

@labkode labkode left a comment

Choose a reason for hiding this comment

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

Changes applied

@labkode labkode requested a review from rdimaio July 19, 2024 08:36
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
docs/started/daemons.md Outdated Show resolved Hide resolved
@labkode
Copy link
Member Author

labkode commented Dec 3, 2024

@voetberg addressed your suggestions.

@labkode labkode force-pushed the daemons branch 2 times, most recently from af9c7ff to 6decc10 Compare December 3, 2024 14:06
@labkode labkode requested a review from rdimaio December 3, 2024 14:25
@labkode
Copy link
Member Author

labkode commented Dec 3, 2024

@voetberg @rdimaio now the build fails for no apparent reason nor directly related to my changes, I've re-started but still fails, can you take a look?

@rdimaio
Copy link
Contributor

rdimaio commented Dec 3, 2024

ModuleNotFoundError: No module named 'rich'

seems to be related to this PR that just got merged rucio/rucio#7012

@rdimaio
Copy link
Contributor

rdimaio commented Dec 3, 2024

we need to add rich here: https://github.com/rucio/documentation/blob/main/tools/requirements.txt - i'm making a PR now

@voetberg
Copy link
Contributor

voetberg commented Dec 3, 2024

we need to add rich here: https://github.com/rucio/documentation/blob/main/tools/requirements.txt - i'm making a PR now

You're way ahead of me, I was just putting one in myself lol

@rdimaio
Copy link
Contributor

rdimaio commented Dec 3, 2024

PR to add dependency: #407
but it's failing with the same error.

This is happening right after it pulls the cached test image:

Status: Downloaded newer image for ghcr.io/rucio/rucio/rucio-autotest:alma9-python3.9
Generating documentation files...
Generating the Rest Api Docs...
Generating the Executable Help Docs...
Traceback (most recent call last):
  File "/run_in_docker/rucio/bin/rucio", line 35, in <module>
    from rich.console import Console
ModuleNotFoundError: No module named 'rich'

which was last published 12 hours ago: https://github.com/rucio/rucio/pkgs/container/rucio%2Frucio-autotest

I just started a new cache build run: https://github.com/rucio/rucio/actions/runs/12142963668, should take 20 mins. After it's built, I'd say:

  • let's try rerunning the build step in this PR (346) just to see if it passes without needing to add rich as dep in rucio/documentation as well
  • if it doesn't pass, let's try rerunning the build step in Dependencies: Add rich; fix #406 #407

@labkode
Copy link
Member Author

labkode commented Dec 4, 2024

@rdimaio build failed, trying adding rich now.

@labkode
Copy link
Member Author

labkode commented Dec 4, 2024

@rdimaio @voetberg still fails. Can some of you revert the PR that introduced the breaking change so other PRs can go through?

@voetberg
Copy link
Contributor

voetberg commented Dec 4, 2024

@rdimaio @voetberg still fails. Can some of you revert the PR that introduced the breaking change so other PRs can go through?

The problem isn't from docs, it's from the main repo. This commit - rucio/rucio@d58adca
It added a dependency not present in the image used for generated the client api docs

@rdimaio
Copy link
Contributor

rdimaio commented Dec 4, 2024

@rdimaio @voetberg still fails. Can some of you revert the PR that introduced the breaking change so other PRs can go through?

The problem isn't from docs, it's from the main repo. This commit - rucio/rucio@d58adca It added a dependency not present in the image used for generated the client api docs

the error is different now that rich has been added: https://github.com/rucio/documentation/actions/runs/12142845622/job/33913473175?pr=407

@rdimaio
Copy link
Contributor

rdimaio commented Dec 4, 2024

@labkode the issue will be solved by this PR #409 - in the meantime, you can remove the rich dependency change from this PR as it is not needed anymore

@labkode
Copy link
Member Author

labkode commented Dec 5, 2024

@rdimaio all good now thanks!

@labkode labkode dismissed rdimaio’s stale review December 5, 2024 08:33

removed rich

@labkode labkode merged commit e49dc41 into rucio:main Dec 5, 2024
11 of 12 checks passed
@labkode labkode deleted the daemons branch December 5, 2024 08:33
@labkode labkode restored the daemons branch December 5, 2024 10:19
@labkode
Copy link
Member Author

labkode commented Dec 5, 2024

@rdimaio I thought it was green as the "Merge" button appeared but the build is still failing.

@rdimaio
Copy link
Contributor

rdimaio commented Dec 5, 2024

@rdimaio I thought it was green as the "Merge" button appeared but the build is still failing.

CI looks good here https://github.com/rucio/documentation/actions/runs/12176030045

@voetberg
Copy link
Contributor

voetberg commented Dec 5, 2024

@rdimaio I thought it was green as the "Merge" button appeared but the build is still failing.

The CI was running off your branch and not the main repo, so the failed doc building was still present. The fixes in main solve the build problem when it merges, but not in the PR itself.

@labkode
Copy link
Member Author

labkode commented Dec 6, 2024

@voetberg what else is missing for these changes appear in the main documentation portal?
https://github.com/rucio/documentation/

@voetberg
Copy link
Contributor

voetberg commented Dec 6, 2024

@labkode They're present? https://rucio.cern.ch/documentation/started/daemons

If you mean the sidebar, it looks like your pr was missing adding them to website/sidebars.json (we should have caught that in review.) We can open another PR to fix that.

@rdimaio
Copy link
Contributor

rdimaio commented Dec 6, 2024

just realized that these daemons are under the started directory - was that on purpose?

edit: my bad, that's the "Getting started" docs directory, so I think it fits (could argue it might go into the Operator docs instead but I think it's fine in the current section)

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.

Make a table for all daemon and its functionality
4 participants