Azure / azure-functions-durable-python

Python library for using the Durable Functions bindings.
MIT License
136 stars 55 forks source link

Incorrect type hint for content parameter of call_http in DurableOrchestrationContext.py #518

Open louic-vermeer-pggm opened 2 months ago

louic-vermeer-pggm commented 2 months ago

The content parameter in the call_http function in DurableOrchestrationContext.py has type hint Optional[str] but actually accepts any json serializable content, see lines 238-242. The type hint should be corrected.

In addition, the conditional if content and content is not isinstance(content, str) is hard to read and should probably be rewritten as if content and not isinstance(content, str).

Expected behaviour: when passing json serializable content, the type checker should not complain

davidmrdavid commented 1 month ago

Thanks @louic-vermeer-pggm. I'm open to improving the types here but first, last I checked - there was no Python type hint for "json serializable", has that changed? I realize Optional[str] is overly restrictive, but I'm curious if there's a more general (but still precise) type hint we could be using here.

louic-vermeer-pggm commented 1 month ago

As far as I know there is no consensus on a good type hint for the json format, so I understand the choice for the restrictive Optional[str]. We may have to wait until python or its type checkers provide a solution. In the meantime I think it would at least help if the documentation was updated to indicate that json serializable content is also allowed despite the type hint.

davidmrdavid commented 1 month ago

I agree with this notion. For now, I'll be labeling this as 'good first issue' and 'help wanted' in case the community would like to contribute the documentation fix. We core team has a few bug fixes to tackle before we can get to this, so contributions are welcomed.