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.

Sign up for GitHub

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

Jump to bottom

Update Watcher documentation in README #363

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

Update Watcher documentation in README #363

wants to merge 28 commits into from

Conversation

mbthornton-lbl
Copy link
Contributor

Update the Setting up Watcher/Runner on Perlmutter section of the project README

@mbthornton-lbl mbthornton-lbl linked an issue Jan 24, 2025 that may be closed by this pull request
Update README for running Watcher and Scheduler #362
Open
kaijli
kaijli requested changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@kaijli kaijli left a comment

Choose a reason for hiding this comment

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

Thanks for updating the documentation. I think if this were read by someone just getting started with trying to run automation or coming back from not touching it for a while (the rest of us), it's still not very self explanatory. It would be great if it could be formatted differently as well to be more clear about which are commands and which are instructions, comments, or explanations.

@kaijli
Copy link
Contributor

kaijli commented Jan 24, 2025

I'm reading through the documentation updates again after leaving comments and I think it should be structured differently, because on the first read through I would think that it's steps to get it running with parts missing, but on the second read (and background understanding of how it runs), I can see that it's different sections starting from summary to steps. Could be more clear in that aspect through formatting.

@kaijli
Copy link
Contributor

kaijli commented Jan 24, 2025

Also a typo here:

nmdc_automation/README.md

Line 27 in bba3fe4

* Inputs: The array of inputs for the workflow. Not it doesn't deal with nested structures yet.

"Not" instead of "Note"

@kaijli
Copy link
Contributor

kaijli commented Jan 24, 2025

Why is this section crossed out instead of just deleted or commented out

nmdc_automation/README.md

Lines 58 to 60 in bba3fe4

~~`nmdc_automation/run_process/run_worklfows.py` will automate job claims, job processing, and analysis record and data object submission via the nmdc runtime-api.~~
~~To submit a process that will spawn a daemon that will claim, process, and submit all jobs that have not been claimed, `cd` in to `nmdc_automation/run_process`
and run `python run_workflows.py watcher --config ../../configs/site_configuration_nersc.toml daemon`, this will watch for omics processing records that have not been claimed and processed.~~

aclum
aclum reviewed Jan 24, 2025
View reviewed changes
aclum
aclum reviewed Jan 29, 2025
View reviewed changes
aclum
aclum reviewed Jan 29, 2025
View reviewed changes
aclum
aclum reviewed Jan 29, 2025
View reviewed changes
@mbthornton-lbl mbthornton-lbl requested a review from aclum January 29, 2025 19:16
kaijli
kaijli reviewed Feb 7, 2025
View reviewed changes
Copy link
Contributor

@kaijli kaijli left a comment

Choose a reason for hiding this comment

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

a couple more questions about the content

@aclum
Copy link
Contributor

aclum commented Feb 19, 2025

@mbthornton-lbl my review comments from several weeks ago still have not been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aclum aclum Awaiting requested review from aclum

@kaijli kaijli Awaiting requested review from kaijli

@vlilanl vlilanl Awaiting requested review from vlilanl

@AmitBinf AmitBinf Awaiting requested review from AmitBinf

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update README for running Watcher and Scheduler
3 participants
@mbthornton-lbl @kaijli @aclum