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

Webapi spec update #250

Closed joschrew closed 7 months ago

joschrew commented 11 months ago

I think this PR is currently nothing more than a draft and a call for discussion. At least for me some things are not clear and I cannot just update it without further information. This PR contains some commits all for the same document. With this I tried to make more clear what I wanted to change and what was intended.

Here are my open questions: General questions:

More detailed question regarding the current texts:

Questions more related to the webAPI in general:

tdoan2010 commented 11 months ago

What is our goal with this document?

It should contain a detailed description about the Web API, e.g., how it was implemented, how should it be used. Don't worry that it's too much.

Do we want to describe the implementation in core?

Yes. We can describe it in word, or better, with activity diagrams.

Do we want to describe the workflow endpoint we are currently developing?

Yes.

And don't think messages are re-enqueued.

Why not? If somehow a problem occurs during the processing, the message should be re-queued so that the Worker can try processing it again. If ACK is not sent, or a NACK is set, then the message will be re-queued.

What about the result queue?

Just leave it there. It's already implemented. People can decide which approach they want to use, result queue or callback.

Our workflow endpoint currently does not fit to the webAPI. In core we could think about changing that or changing the webAPI to fit to the endpoint.

Yes, the openapi.yml file is completely outdated. Please update it to the current implementation in the code.

We could implement the remainder of the workflow server in core.

Theoretically yes, but let's discuss that later.

joschrew commented 11 months ago

Thanks for the answer. I have added the activity diagrams for worker and processing-endpoint.

Regarding re-enqueue: the code regarding this is channel.basic_nack(delivery_tag=delivery_tag, multiple=False, requeue=False). The messages are currently not re-enqueued due to requeue=False but I will ask about that in our next Monday-meeting before changing the spec.

joschrew commented 10 months ago

I have updated the activity diagram for the Processing Server and the webapi-openapi-spec. To the webapi I rather added unfinished work (TODO comments), because there are some questions left which I wasn't able to answer on my own. So this needs further discussions in one of our meetings on Monday probably.

tdoan2010 commented 9 months ago

@joschrew please add the command to start a Processor Server under section 6.5.

joschrew commented 9 months ago

I updated the path in the web_api.md to the recent changes to the openapi.yml and I changed some text in the Terminology section related to these changes (only text changes in that 6 lines). Additionally the Images web-api-distributed.jpg and web-api-distributed-queue.jpg have a path inside which does not fit any more. It says POST /processor/{executable} but should bePOST /processor/run/{executable}. @tdoan2010, please update this two images if possible. I have read through the web_api.md and I think it is good now and I would change this PR to "ready to be merged" if you think so too ...

tdoan2010 commented 9 months ago

I've updated the path as you asked.