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.

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

chore: change webhook payload structure #397

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

bthari
Copy link
Contributor

@bthari bthari commented Nov 13, 2024

Summary

Changing the structure of the payload send from Turing to a webhook call, so we have the same structure across Merlin and Turing. From

{
    "event_type": "on-model-endpoint-created",
    "data": {} // the data being either ensembler/router/router_version 
}

to

{
    "event_type": "on-model-endpoint-created",
    "data": {
        "ensembler": {}, // omit if empty
        "router": {}, // omit if empty
        "router_version": {} // omit if empty
    }
}

I make the payload with this structure in case in the future we want to put several object in the payload (instead of just one object like now). I’m open to any advice if you have other opinions

Copy link
Contributor

@deadlycoconuts deadlycoconuts left a comment

Choose a reason for hiding this comment

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

Ah oops I didn't know you needed it to be in exactly the same schema as the request body in Merlin, my bad. Thanks for fixing this!

@bthari
Copy link
Contributor Author

bthari commented Nov 13, 2024

No worries @deadlycoconuts! It's minor and simply for consistency matter, so user will not be confused why we have different schema, and I also just remember it this morning. Thank you for the quick review!

@bthari bthari merged commit 39865c0 into main Nov 13, 2024
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: maintenance Chore changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants