Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

webhook: Add initial webhook plugin #39

Merged
merged 6 commits into from
Feb 7, 2024
Merged

webhook: Add initial webhook plugin #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5c98851
Add minimal webhook implementation
jcharum Feb 5, 2024
4df7212
Refine types and docstring
jcharum Feb 5, 2024
8f029be
Fix async usage
jcharum Feb 5, 2024
82a83d7
Correct tense of docstring
jcharum Feb 5, 2024
0ceaf42
Refine docstring
jcharum Feb 5, 2024
1ad1ca7
Delete logo line
jcharum Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions webhook/plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from typing import Any

import httpx


async def webhook_request(url: str, body: Any) -> Any:
Copy link
Collaborator

@bozhi bozhi Feb 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why the body's type and return type is Any? I think the type checker will not catch any issues here and that seems a little problematic.

Copy link
Collaborator Author

@jcharum jcharum Feb 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, body and the return value are arbitrary data structures that can be encoded to and decoded from JSON, respectively. There are enough trade-offs around what that type should even be that Python does not provide one in the type hints of the standard library. See the discussion in this issue. The final comment summarizes nuances around type-checking. Given the nuance, particularly around invariance and mutability, and the generality of webhook calling, it's not obvious to me what trade-off to make.

Similarly, the json parameter and return value of .json() are Any, presumably for the same reasons.

Furthermore, plugins do not yet support unions, as unions are generally very prone to introduce ambiguity in deserialization, and there is no mechanism to disambiguate. The one that exists for built-in actions is pretty fragile and requires some relatively careful thinking, so I am resisting adding support to plugins until we have a very very good reason.

Do you have an example of a problem case, as I think a worked example would be useful to explore these trade-offs?

"""
Make a POST request to the given URL with the given body and returns the response.

Both the request and response bodies are expected to be JSON and are converted to
and from Python values using the json module.
"""
async with httpx.AsyncClient() as client:
return (await client.post(url, json=body)).raise_for_status().json()
2 changes: 2 additions & 0 deletions webhook/plugin.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
name = "Webhook"
description = "Use external webhooks."