feat: initial typing of the public API #248
Conversation
This adds type annotations for the 2 main decorators of the library. Closes: #190
| ```python | ||
| import flask | ||
| import functions_framework | ||
|
|
||
| @functions_framework.http | ||
| def hello(request): | ||
| def hello(request: flask.Request) -> flask.typing.ResponseReturnValue: | ||
| return "Hello world!" | ||
| ``` |
There was a problem hiding this comment.
I'm not sure how to best show this to users:
- On one hand, I think it would be great if developers don't have to search for a long time the type definitions of the functions
- On the other hand, I really like these short examples in the README file
🤔
There was a problem hiding this comment.
Including these type annotations for the sake of illustration in this developer focused README.md seems like a reasonable doc update, some of the essence of the simplicity of this sample is preserved on the GCP docs in https://cloud.google.com/functions/docs/samples/functions-helloworld-get#functions_helloworld_get-python.
setup.py
Outdated
| package_data={"functions_framework": ["py.typed"]}, | ||
| zip_safe=False, |
There was a problem hiding this comment.
There was a problem hiding this comment.
Good find, but I'm now seeing that this recommendation has been removed. python/mypy#8802 (comment). We might be safe to remove explicitly setting zip_safe. WDYT?
There was a problem hiding this comment.
You are right, I removed the flag 👍
| CloudEventFunction = Callable[[CloudEvent], None] | ||
| HTTPFunction = Callable[[flask.Request], flask.typing.ResponseReturnValue] |
There was a problem hiding this comment.
The signature of the HTTP function is a bit verbose :/
Also, if the type checking fails, the error message is ... interesting :)
$ cat main.py
import functions_framework
import flask
@functions_framework.http
def hello(request: flask.Request) -> flask.typing.ResponseReturnValue:
return False
$ mypy --strict main.py
main.py:7: error: Incompatible return value type (got "bool", expected "Union[Union[Response, str, bytes, List[Any], Mapping[str, Any], Iterator[str], Iterator[bytes]], Tuple[Union[Response, str, bytes, List[Any], Mapping[str, Any], Iterator[str], Iterator[bytes]], Union[Headers, Mapping[str, Union[str, List[str], Tuple[str, ...]]], Sequence[Tuple[str, Union[str, List[str], Tuple[str, ...]]]]]], Tuple[Union[Response, str, bytes, List[Any], Mapping[str, Any], Iterator[str], Iterator[bytes]], int], Tuple[Union[Response, str, bytes, List[Any], Mapping[str, Any], Iterator[str], Iterator[bytes]], int, Union[Headers, Mapping[str, Union[str, List[str], Tuple[str, ...]]], Sequence[Tuple[str, Union[str, List[str], Tuple[str, ...]]]]]], Callable[[Dict[str, Any], StartResponse], Iterable[bytes]]]") [return-value]
Found 1 error in 1 file (checked 1 source file)
I reckon it's more a Flask issue, would you have an idea how to make that better?
There was a problem hiding this comment.
Is return False something that we'd expect to work? Testing with the latest version of ff I think an error is issued when attempting to return a bool.
There was a problem hiding this comment.
I don't remember exactly why I returned False in this example, I think the idea was just to return a non-valid type to see what the type checker would show in this case.
So basically, if a developer returns a non-valid type from a @functions_framework.http handler, this will be the error show to the developer.
|
Thanks @multani for contributing this pull request! Sorry for the delay in getting a review out for these changes. This is a contribution we'd be happy to merge and your work looks like a great step towards adding typing for functions framework python. |
|
Thanks! Would you mind adding one more commit with the output of to appease the linter. You may need to install the black code formatter if you don't already have it. |
|
@garethgeorge I commited the change from |
|
Hi @multani , thanks! It looks like there are still some failing lints (import order I believe) would you mind taking a look? |
This adds type annotations for the 2 main decorators of the library.
Closes: #190
I'm also carrying over a small stub file for the event-based functions, which would be now available for everybody.