Skip to content

Fix ocrd_network client run argument order#1303

Merged
kba merged 6 commits intoOCR-D:masterfrom
bertsky:fix-ocrd-network-client-run-args
Feb 11, 2025
Merged

Fix ocrd_network client run argument order#1303
kba merged 6 commits intoOCR-D:masterfrom
bertsky:fix-ocrd-network-client-run-args

Conversation

@bertsky
Copy link
Copy Markdown
Collaborator

@bertsky bertsky commented Feb 5, 2025

No description provided.

@bertsky bertsky requested a review from MehmedGIT February 5, 2025 13:46
@bertsky
Copy link
Copy Markdown
Collaborator Author

bertsky commented Feb 5, 2025

@kba should I make a v2 backport of 0d277ac?

I should probably create an issue for this, but perhaps someone can get me on track here: Shouldn't we also allow the user to pass the ocrd_utils.config options to the server, somehow? (In v3, you'll remember, we introduced fine-grained controls like OCRD_MISSING_INPUT, OCRD_MISSING_OUTPUT, OCRD_EXISTING_OUTPUT, that should in principle be configurable on a case-by-case basis. Without any such delegation, one would have to live with the defaults set on the server side, so there's not much use for them in the network scenario...)

@bertsky
Copy link
Copy Markdown
Collaborator Author

bertsky commented Feb 6, 2025

I should probably create an issue for this, but perhaps someone can get me on track here: Shouldn't we also allow the user to pass the ocrd_utils.config options to the server, somehow? (In v3, you'll remember, we introduced fine-grained controls like OCRD_MISSING_INPUT, OCRD_MISSING_OUTPUT, OCRD_EXISTING_OUTPUT, that should in principle be configurable on a case-by-case basis. Without any such delegation, one would have to live with the defaults set on the server side, so there's not much use for them in the network scenario...)

Another problem is with how parameter resources get resolved now. In the CLI setting, we allowed an ambiguity between JSON literals and file names, so the latter would have to be resolved to absolute paths (for the various resource locations), which in turn obviously dependend on the processor class (because of the custom module location):

if 'parameter' in kwargs:
# Disambiguate parameter file/literal, and resolve file
def resolve(name):
try:
return processor.resolve_resource(name)
except ResourceNotFoundError:
return None
kwargs['parameter'] = parse_json_string_or_file(*kwargs['parameter'],
resolve_preset_file=resolve)

But in the network setting, where parameters can be added to the processing or workflow request, no such resolution (from resource name to resource path) takes place –

  • On the server side, there is only a parameter validation step
    report = ParameterValidator(ocrd_tool).validate(dict(job_input.parameters))
  • On the client side, there is only the JSON string vs file disambiguation, but no prior resolution.
    req_params["parameters"] = set_json_key_value_overrides(parse_json_string_or_file(*parameter), *parameter_override)

Did we abandon the entire mechanism, or am I missing something?

@kba
Copy link
Copy Markdown
Member

kba commented Feb 10, 2025

@kba should I make a v2 backport of 0d277ac?

It depends - does the previous behavior trigger a bug? AFAICT we're only calling the method with kwargs or am I missing something?

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.

3 participants