- 
                Notifications
    You must be signed in to change notification settings 
- Fork 76
ops: jobs: new jobs docs #784
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, a couple things to fix :)
|  | ||
| ## Why a Job System? | ||
|  | ||
| Traditionally, Celery tasks in InvenioRDM were launched programmatically or via CLI. This created challenges: | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this sentence is a bit strange to me, because usually we didn't run celery tasks by cli, in majority of cases celery tasks were launched by the application or cron
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that majority of cases but I have trigered a fet times the user_sync task via CLI, otherwise there was no other way.
        
          
                docs/maintenance/internals/jobs.md
              
                Outdated
          
        
      |  | ||
| ## Logging & Auditability | ||
|  | ||
| If a Celery task logs via `current_app.logger`, and is invoked as a job, its logs are captured and stored with the run metadata. These logs are shown in the admin UI. All logs are stored in OpenSearch and displayed via the job run UI and API. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part looks to me more as a admin panel user documentation, not the maintenance description for jobs module. For maintenance it would be more relevant to explain how the log line ends up in the report (how to call logger and when - for different log levels)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm to me this looks like on the right place mainly because we are explaining here how to create those logs, it's said in the first sentence
If a Celery task logs via
current_app.logger, and is invoked as a job, its logs are captured and stored with the run metadata.
Since this is internal docs (an explanation) I don't think we need more than what is described to let the user know that they can benefit from having logs by using the app logger. I think explaining the different log levels is should be trivial to any user reading this docs.
Explaining more about how the logging works would exceed the purpose of this doc and overwhelm the user with not essential information about the jobs. Happy to discuss more about this IRL in case I am missing something.
P.S. Also we are already showing an example on how to log in the how to guide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
End up being a bit more verbose around this, let me know if it's clearer now!
| Jobs can be: | ||
|  | ||
| - Run immediately from the UI. | ||
| - Scheduled with CRON or interval-based configurations via the admin panel. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems to me also that could be a part of admin user documentation, not maintenance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO it should be mentioned in different places with different purpose, in the admin user documentation it should and is mentioned with a screenshot, here is simply explained.
| @@ -0,0 +1,107 @@ | |||
| _added in v13.0.0_ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(note for placement discussion)
This page should be in "Use" (/use/jobs) bc purely administration panel interactions. Reference links to the configure page can be added to it.
Co-authored-by: Carlin MacKenzie <carlin.mackenzie@gmail.com>
replace user with instance manager/developer. Co-authored-by: Karolina <38131488+kpsherva@users.noreply.github.com>
Co-authored-by: Carlin MacKenzie <carlin.mackenzie@gmail.com>
Creating this PR to collaborate on Jobs documentation