Base process manager does not allow a processor to return bytes as the generated output anymore

[ricardogsilva]

If using a builtin process manager, such as the tinydb, mongo or postgresql ones, which all delegate to the base manager's _execute_handler_sync() at some point, when an executing process generates bytes as its output, the process manager converts it to a UTF-8 string, as seen in:

pygeoapi/pygeoapi/process/manager/base.py

Lines 280 to 281 in b9e81dd

	if isinstance(outputs, bytes):
	outputs = outputs.decode('utf-8')

I guess this means that support for binary output types (for example, a geotiff file) is now broken?

Further down in the same method, there is still a check for bytes, as per:

pygeoapi/pygeoapi/process/manager/base.py

Lines 301 to 304 in b9e81dd

	elif isinstance(outputs, bytes):
	mode = 'wb'
	data = outputs
	encoding = None

but the code inside this if condition is now not reachable, because the type of outputs has been modified to a str above.

@tomkralidis this seems like a regression introduced in this recent change: 0b53d49

[francescoingv]

@ricardogsilva
Please have a look at https://github.com/francescoingv/ingv-pygeoapi-process-plugins/tree/main/ingv_plugin_pygeoapi/process

PyboxProcessor in pybox.py may return geotiff.

It can be tested at site:
https://voice.pi.ingv.it/geoinquire
specifically:
https://voice.pi.ingv.it/geoinquire/processes/pybox

For a curl request please look at examples in json process description

Please let me know of any problem.

[ricardogsilva]

@francescoingv I took a look at your code and it seems to me that you never return bytes from your processor's execute() method, as you perform some base64 encoding of the geotiff - therefore your implementation is not affected by the issue described above.

If I am mistaken, please let me know, but to the best of my knowledge this is indeed a regression in pygeoapi

[francescoingv]

@ricardogsilva according to the standard, bytes returned inline must be as strings encoded byte64.

I intrpreted that the processor should return that string.

Differently, for output returned by reference, the reference point to byte streams.

If my "interpretation" is wrong please advise.

[ricardogsilva]

@francescoingv please refer specifically to the part of the standard, so that it is easier to discuss 😉

What is in the standard:

• https://docs.ogc.org/is/18-062r2/18-062r2.html#binary-value - Inputs may be specified inline, i.e. as JSON property values - for binary formats it is only possible to do this if you indeed base64 encode them, as this is a suitable encoding for transmitting binary data over a text representation
• In 'clause 7.13.2', specifically the example: 'Example 2 — A result encoded as JSON.' you can see again a base64 encoded example of an output. But this is because the example is a result encoded as JSON. If you are returning a JSON document as your result then yes, you need to base64 encode any binary outputs.

The standard also summarizes, in Table 12, that you are allowed to return a result type of raw, in which case you can use whatever media type is best suited for the output. In our hypothetical example, if I'm returning a raw geotiff file then I would only need to specify its media type and dump the binary bytes as the response body, without needing to convert to base64.

[francescoingv]

@ricardogsilva
Effectively the only reference I found about output is the following:

6.1.  Encodings
JSON is the encoding for requests and responses. The inputs and outputs of a process can be any format. The formats are defined at the time of job creation and are fixed for the specific job.

Thereafter the only specification I found on in-line requirements are about input.
I interpreted the same is valid for output, as in my opinion (but again I didn't find it stated anywhere) output should be able to be provided as input for matching definitions.

Said that,
as I see the code now, there are still undefined cases:

1. Processor.execute() returning a number by value
2. Processor.execute() returning a boolean by value

If Processor.execute() would return outputs as bytes and the Manager does not convert it to a UTF-8 string,
then I wonder what happens upstream (i.e. in the API layer) when outputs is instance of bytes:

3. if "document" is required
4. if "raw" is required

[ricardogsilva]

1. Processor.execute() returning a number by value
2. Processor.execute() returning a boolean by value

For outputs composed of these simple types, then application/JSON seems very suitable as a raw representation, but I'd argue the logic for serializing the outputs should be handled in the processor and not in the process manager - this is a matter for other discussion (see #2305), not this current issue.

The second part of your comment is also something I address in #2305. The scope of this issue is merely to describe a change in behavior in the pygeoapi process manager, which I think is a regression, let's not expand the scope.

[francescoingv]

If I understood it correctly,
your present issue is aimed to remove the lines

 if isinstance(outputs, bytes): 
     outputs = outputs.decode('utf-8') 

which have side effects, as per my second part.

I agree this issue is related to #2305
and possibly a better (or reviewed) definition of what Processor.execute() should return would address both of them. I mean should it either return:

• the list of output items required by the client
• the body of the output

Possibly the above should be part of #2305, or a new issue should be opened.