-
Notifications
You must be signed in to change notification settings - Fork 3
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
refactoring and CLI for presentation #35
Conversation
Note: for adaptation on the Kitodo.Production side, you will need the following patch: --- _resources/data/scripts/script_ocr.sh 2022-06-30 15:36:12.000000000 +0200
+++ _resources/data/scripts/script_ocr.sh 2022-09-27 12:23:11.495580732 +0200
@@ -16,11 +16,12 @@
exit 3
fi
-COMMAND="for_production.sh $PROCESS_ID $TASK_ID /data/$PROCESS_ID deu Fraktur true"
+COMMAND="for_production.sh --proc-id $PROCESS_ID --task-id $TASK_ID --lang deu --script Fraktur"
WORKFLOW_FILE="ocr-workflow.sh"
if test -f "/usr/local/kitodo/metadata/$PROCESS_ID/$WORKFLOW_FILE"; then
- COMMAND+=" /data/$PROCESS_ID/$WORKFLOW_FILE"
+ COMMAND+=" --workflow /data/$PROCESS_ID/$WORKFLOW_FILE"
fi
+COMMAND+=" /data/$PROCESS_ID"
logger -p user.notice -t $TASK "ssh destination 'ocrd@$OCRD_MANAGER_HOST' port '${OCRD_MANAGER_PORT:-22}' running command '$COMMAND'"
(this will then have to go into data.zip again) |
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 looks very good! I'll then test the changes at least for Production against our stack (with changes from above) and then I'll merge if it works.
In the orcd_lib.sh script I suggest to name functions and values perspectively independent from our applications Kitodo.Production and Kitodo.Presentation and use functional naming. But we could also do this later in a separate issue / PR so that it does not become too much at once.
e.g.
parse_args_for_presentation
-> parse_args_for_mets_processing
parse_args_for_production
-> parse_args_for_procdir_processing
For closing the task in Kitodo.Production I can imagine a seperated lib kitodo-production_lib.sh. This contains all application-specific functions come.
Afterwards the manager supports Kitodo.Production but the core functionality is independent and can be used with any application.
Yes, I thought of doing some of these name changes within this PR already (so we would only need to update the OCR script once). And we could indeed support more use cases than just Kitodo. But then again, we advertise this endeavor as Kitodo integration – why not heed our target users' naming expectations, and stay specific to the purpose? Nobody prevents other users from adapting the shell scripts to their own needs... |
Mainly I am concerned about the naming in the script ocrd_lib.sh. I rather saw this lib as a fixed. So the user does not have to make any adjustments here. I think it would be good to keep the applications out of the lib cause of updateability of the ocrd_lib.sh script when updating OCRD Manager image. So that only adjustments in the scripts which use the library must be made. |
I see – agreed! Then lets move the arg parsers from ocrd_lib.sh to the top-level scripts. |
Note: the last commit fixes slub/ocrd_kitodo#40 |
This is a first working version for an interface from METS to METS.
We might still need fiddle with the details (both in interface definition and implementation).
(One of the problems I saw was that ocrd_browser cannot show workspaces which still have remote images in them, see here.)