[wip] Implement webhooks API

[lappemic]

Hey @Wauplin, here's the initial draft for #1808, trying to align with #1905 as you suggested. However, a couple of points are still unclear to me, and I'd love to get your input to ensure the implementation aligns correctly:

• Could you clarify what the secret parameter represents in create_webhook and update_webhook methods? Is it intended to be the user token?
• Regarding the definition of DOMAIN_T Literal, would it be appropriate to define it directly within the function signatures?

I plan to finalize docstrings, module imports, and tests once we're on the same page regarding these questions.

Thanks a lot for your guidance and feedback!

[HuggingFaceDocBuilderDev]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

[Wauplin]

Thanks @lappemic!

I didn't have a look yet to your changes but to answer your question:

• the secret correspond to a value set by the user and that will be sent in every webhook. This way the user is sure the webhook is sent by the Hub. So this is not the same as token no. You can have more info here: https://huggingface.co/docs/hub/webhooks
• if it's used only once, then you don't need to define DOMAIN_T explicitly. If used more than once, yes it's best to define it (maybe called WEBHOOK_DOMAIN_T).

Also 2 comments about the changes:

• it's best to define a dataclass rather than a typed dict as return value
• there are a lot of changes due to styling. Please run pip install -e '.[dev]' from your local folder to install all the required deps and then run make style to comply with huggingface_hub standard. You can run make quality to check everything's fine.

[lappemic]

Thanks for the quick response, @Wauplin!

1. Understood about the secret; it's clear now.
2. Since WEBHOOK_DOMAIN_T is utilized more than once, I'll go ahead and define it in constants.py. Does that sound right?
3. I'm a bit uncertain about your suggestion to prefer a dataclass over a TypedDict for return values. Could you specify which part of my changes this refers to? I would think it is abou the WatchedItem definition to do rather like this

@dataclass
class WatchedItem:
    """Data structure containing informatin about webhooks watched for updates.

    Attributes:
        type (`Literal["dataset", "model", "org", "space", "user"]`):
            Type of the item to be watched. Can be one of `["dataset", "model", "org", "space", "user"]`.
        name (`str`):
            Name of the item to be watched. Can be the username, organization name, model name, dataset name or space name.
    """

    type: Literal["dataset", "model", "org", "space", "user"]
    name: str

4. Regarding the style changes, I had previously run make style and make quality thinking they'd align my code with Hugging Face's style guidelines. However, it seems some styling issues persist. I've run both commands again without any changes. Any clue what might be causing this?

Appreciate your guidance and looking forward to your thoughts!

[Wauplin]

Since WEBHOOK_DOMAIN_T is utilized more than once, I'll go ahead and define it in constants.py. Does that sound right?

Yes!

I would think it is about the WatchedItem definition to do rather like this

Exactly! That will be easier to use for users downstream

Regarding the style changes, I had previously run make style and make quality thinking they'd align my code with Hugging Face's style guidelines. However, it seems some styling issues persist. I've run both commands again without any changes. Any clue what might be causing this?

Hmm that's weird. I would assume something's off with the install. Are you using a virtualenv? If no, I'd advice to create a new one (installing Python3.9 or 3.10 in it) and install the dependencies in it. You can check install guidelines here: https://huggingface.co/docs/huggingface_hub/installation#install-with-pip (you'll need an editable install from source in a virtual env)

[lappemic]

Thank you for the feedback!

I've updated the PR with the latest commits, according to your suggestions.

Regarding the styling issues, I initially followed the steps outlined in the repo's CONTRIBUTING.md without success. Then, I set up a new virtual environment and followed the installation instructions from https://huggingface.co/docs/huggingface_hub/installation#install-with-pip, which worked now. The only difference is the `[dev]' notation. I assume this should not make any difference?

Looking forward to your review and approval to continue.

[Wauplin]

Thanks @lappemic! I'll have a closer look to it when I have time later this week. It seems that the styling still have some issues (see here) but I can take care of that if you want. Thanks again!

[Wauplin]

Thanks for the PR @lappemic! I just reviewed it and made high level comment to it. The structure is good and I think you can continue on that. Sometimes I commented on a specific method but the comment can be applied to all comment.

As you can see, I also pushed commits to fix styling stuff. Make sure not to push changes that are unrelated to your PR, otherwise it makes the review very difficult (I have to "check" if a change is related to your PR or to some styling). If you correctly install the [dev] dependencies + run make style, it shouldn't happen. Anyway, let me know if you have questions related to this. And don't forget to git pull before starting to make changes.

[lappemic]

Thanks you very much for the proper review and inputs @Wauplin. I hope to find time to incorporate everything this week. It might take up to next week though.

[Wauplin]

Thanks @lappemic! And no worries about the timing, days are always too busy 😄

[lappemic]

Hey @Wauplin, found some time to implement your feedback.

I was not quite sure if enable_webhook and disable_webhook also should return the Webhook? I assumed yes, so the users gets instant feedback.

As soon as the main changes are alright, i will implement the tests and extend the documentation accordingly.

PS. Sorry for the style mess in the previous commits!! 🙈

[Wauplin]

Hi @lappemic, I've made a more thorough review and commented some parts related to arguments order and default values.

[lappemic]

Hi @Wauplin,

I have included the changes based on your feedback and fixed some issues in the last commits:

1. Adjusted keyword order and making them keyword only
2. Standardized the argument order in all WebhookInfo return values and updated corresponding docstrings, including examples
3. Resolved serialization issue in create_webhook and update_webhook
4. Fixed JSON parsing issue
5. Added tests

Additionally I verified the return values of enable_webhook and disable_webhook: Both methods return a value, so I have made no changes made there.

Let me know if this looks good or if i can adjust something. Looking forward to your feedback.

[Wauplin]

Great job @lappemic! Everything looks good implementation-wise now :) I left minor comments but that's all.

Last but not least, could you add some documentation to let users know about this new feature? Here is what I would do:

• rename https://huggingface.co/docs/huggingface_hub/guides/webhooks_server to https://huggingface.co/docs/huggingface_hub/guides/webhooks
• add a first section about how to manage webhooks: create, list, enable/disable, delete. Basically the same information / examples as in the docstring but more in the form of a user guide.
• move all the current content of Webhooks Server inside a Webhook Server section in the same page
• add a redirection from https://huggingface.co/docs/huggingface_hub/guides/webhooks_server to https://huggingface.co/docs/huggingface_hub/guides/webhooks => I show you this once the rest is done. It is to avoid breaking previous urls. (see _redirects.yml.

In practice it means:

• renaming webhooks_server.md to webhooks.md
• updating the toctree.yml
• add content in webhooks.md
• add WebhookInfo and WebhookWatchedItem to the listed dataclasses in the package reference

Please let me know if that's fine for you and if you have any questions. That would make a complete PR for a new feature, from implementation to testing to documenting it. Thanks!

[lappemic]

Thanks a lot @Wauplin!

I would love to finish this off with the added documentation. I ping you asap it's implemented.

[lappemic]

@Wauplin in the last commits i have

• renamed the guides file from webhooks_server.md to webhooks.md
• created the "Managing Webhooks" section in the guides/webhooks.md, moved the server section and rephrased some parts to account for "Server" and "Managing" sections
• added WebhookInfo and WebhookWatchedItem to package_reference
• updated the _redirects.yml (Went ahead and just adopted the style) and _toctree.yml file

I first was confused about the webhooks_server part in the package_reference. But now i think there is no need for changing this part. WDYT?