OCR-D / spec

Specification of the @OCR-D technical architecture, interface definitions and data exchange format(s)
https://ocr-d.de/en/spec/
17 stars 5 forks source link

Web api #222

Closed tdoan2010 closed 1 year ago

tdoan2010 commented 2 years ago

This is my first attempt for the Web API documentation. Comments are welcome.

bertsky commented 1 year ago

Also: could we rename Message QueueJob Queue or Process Queue? (Our system is not about messaging as such, msgqueue is an implementation detail.)

tdoan2010 commented 1 year ago

Is there any concept for a callback sub-API yet? (It's not a good design being forced to poll job status etc. For example, the spec could be extended with some POST /callback with parameters for the server to be registered and the kind of information requested)

To get notified about the job status, one can listen to the job result queue, as described in the result_queue_name part in the spec. So polling is not required.

We can also add the callback feature, but I think listening to the result queue is enough, or?

Also: could we rename Message Queue → Job Queue or Process Queue? (Our system is not about messaging as such, msgqueue is an implementation detail.)

Fine for me. Let's call it Process Queue.

bertsky commented 1 year ago

We can also add the callback feature, but I think listening to the result queue is enough, or?

Help me understand: so the queue is exposed as a Web API endpoint, too? Or is this the regular RabbitMQ API? (In that case, IMO it would help to give a brief explanation on how to use it in our spec, for that particular purpose.)

tdoan2010 commented 1 year ago

No, it's the regular RabbitMQ API. The listener needs to connect to the RabbitMQ and listen to the result queue, as written in the spec:

In case result_queue_name property is presented, the result of the processing will be pushed to the queue with the provided name. If the queue does not exist yet, it will be created on the fly. This is useful when there is another service waiting for the results of processing. That service can simply listen to that queue and will be immediately notified when the results are available.

For example, a job is submitted with result_queue_name: ocrd-binarize-result. Then, to get notified about the job status, your program needs to listen to the ocrd-binarize-result queue.

bertsky commented 1 year ago

Thanks for the explanation!

In that case, IMO it would help to give a brief explanation on how to use it in our spec, for that particular purpose

Or perhaps provide an example of such a request to the RabbitMQ server in that paragraph?

tdoan2010 commented 1 year ago

I've added an example and some explanation in that paragraph. Please take a look :)

bertsky commented 1 year ago

Help me understand: so the queue is exposed as a Web API endpoint, too? Or is this the regular RabbitMQ API? (In that case, IMO it would help to give a brief explanation on how to use it in our spec, for that particular purpose.)

No, it's the regular RabbitMQ API. The listener needs to connect to the RabbitMQ and listen to the result queue, as written in the spec:

It's still strange though that this callback mechanism should be so implementation/Python-specific, and also specific to the queue architecture. What if I just want a REST callback for when my single POST /processor/{exe} call finishes? What if the server in question does not use queues, but directly schedules local CLI calls or distributed Processor REST calls?

tdoan2010 commented 1 year ago

What if the server in question does not use queues, but directly schedules local CLI calls or distributed Processor REST calls?

That's a valid argument for having the callback feature. I will update the spec with this.

tdoan2010 commented 1 year ago

@bertsky could you resolve those conversations which are "done"? 😃 I know that it's still missing a section to describe the standalone REST processors, but I'll add it later, since I don't want to make this PR bigger (it's already big... 😅 ).

bertsky commented 1 year ago

@bertsky could you resolve those conversations which are "done"? I know that it's still missing a section to describe the standalone REST processors, but I'll add it later, since I don't want to make this PR bigger (it's already big... ).

Done. Three points remain open for me, hopefully we can still work these through.

tdoan2010 commented 1 year ago

Done. Three points remain open for me, hopefully we can still work these through.

Sorry, but I only found 2 (the arrow direction and the sync between producers & consumers). What do I miss? 😅

bertsky commented 1 year ago

What do I miss?

the one about an optional POST /workspace for METS URLs – since it's not in the swagger defs yet, and no mention of it being OPTIONAL in the markdown page, I assumed it's still open

tdoan2010 commented 1 year ago

the one about an optional POST /workspace for METS URLs – since it's not in the swagger defs yet, and no mention of it being OPTIONAL in the markdown page, I assumed it's still open

I will update the openapi.yml file after this PR is merged. Then, that endpoint will be there and marked as OPTIONAL, as we discussed.

bertsky commented 1 year ago

Regarding https://github.com/OCR-D/spec/pull/222/files/ad6fe9eb018a40b823161331876f4afaad1827b3..3b2dee9d7c043f61ce04d3369b941f20ec78af2f – I do think sub-grouping is possible. It's been done in core in various places (e.g. ocrd-tool sub-group), too.

tdoan2010 commented 1 year ago

Regarding https://github.com/OCR-D/spec/pull/222/files/ad6fe9eb018a40b823161331876f4afaad1827b3..3b2dee9d7c043f61ce04d3369b941f20ec78af2f – I do think sub-grouping is possible. It's been done in core in various places (e.g. ocrd-tool sub-group), too.

@kba said that it's really complicated to implement sub-command in the OCR-D Click Wrapper. So, better use option.

bertsky commented 1 year ago

@kba said that it's really complicated to implement sub-command in the OCR-D Click Wrapper. So, better use option.

I don't see it. It's just commands inside of groups inside of commands. A good example is ocrd-tool group. Implementation is easy. I could understand if the argument is that it's more complicated to document (esp. in the CLI). But then again, multiple related, interdependent options are also not easy not explain...

kba commented 1 year ago

I don't see it. It's just commands inside of groups inside of commands. A good example is ocrd-tool group. Implementation is easy. I could understand if the argument is that it's more complicated to document (esp. in the CLI). But then again, multiple related, interdependent options are also not easy not explain...

The problem is that we generate the click interface programmatically, so that processor developers only need

@click.command()                                                   
@ocrd_cli_options                                                  
def cli(*args, **kwargs):                                          
    return ocrd_cli_wrap_processor(MyProcessor, *args, **kwargs)

Now, we either require processors to change that to

@click.group()
@ocrd_cli_options
def cli(...):
   return ocrd_cli_wrap_processor(...)

@cli.command('server')
def worker_cli(...):
  return ocrd_cli_worker_subcommand_wrap(MyProcessor)

or find a way to wrap this as a single annotation. It's possible but tricky.

Whereas, for the ocrd command, it's trivial to create subcommands because they use the standard click-way of creating CLI.