Skip to content

feat: svix bridge-health-endpoint #1903

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

Merged
merged 6 commits into from
Apr 29, 2025

Conversation

CodeMan62
Copy link
Contributor

Motivation

There is no health enpoint in svix bridge so this PR adds a health endpoint to svix bridge

Solution

Work in progress

@CodeMan62 CodeMan62 marked this pull request as ready for review April 25, 2025 08:43
@CodeMan62 CodeMan62 requested a review from a team as a code owner April 25, 2025 08:43
Copy link
Contributor

@svix-onelson svix-onelson left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution! 🙏🏻

Looks like there's just one small fixup needed and this can merge through.

@CodeMan62
Copy link
Contributor Author

CodeMan62 commented Apr 29, 2025

@svix-onelson the import is fixed
EDIT:- seems like there are some clippy issues let me fix

Copy link
Member

@svix-jplatte svix-jplatte left a comment

Choose a reason for hiding this comment

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

The START_TIME will only actually be recorded on first use of the health endpoint. Can you add START_TIME.force(); to the start of async fn run so we record it at that point?

@CodeMan62 CodeMan62 force-pushed the svix-bridge-health-endpoint branch from c356cf8 to 66da4bd Compare April 29, 2025 18:00
@CodeMan62
Copy link
Contributor Author

Done @svix-jplatte

@CodeMan62 CodeMan62 force-pushed the svix-bridge-health-endpoint branch from bee7f36 to 3901787 Compare April 29, 2025 18:12
Copy link
Contributor

@svix-onelson svix-onelson left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @CodeMan62 🙏🏻

@svix-onelson svix-onelson merged commit da87aa3 into svix:main Apr 29, 2025
12 checks passed
@CodeMan62
Copy link
Contributor Author

LGTM, thanks @CodeMan62 🙏🏻

Thanks for review 🙏

@svix-mman svix-mman mentioned this pull request May 7, 2025
svix-mman added a commit that referenced this pull request May 7, 2025
* Libs/Python: Bring back the (deprecated) sync `dashboard_access` method, which was accidentally
  removed in v1.64.1
* Libs/Csharp: The `options` argument to the `SvixClient` initializer is now optional.
* Libs/Csharp: The `SvixOptions.BaseUrl` field is deprecated in favor of `SvixOptions.ServerUrl`
* Libs/(Ruby and Kotlin): Add doc comments to class attributes
* Libs/Go: Added a new `<Enum>From<UnderlyingType>` map to all enums. For example `BackgroundTaskStatusFromString["running"]` will result in `BACKGROUNDTASKSTATUS_RUNNING`
* Libs/Go: Fixed bug where the correct `content-type` was not set on `PUT` requests
* Bridge: Add `/health` endpoint by @CodeMan62 in #1903
* Server: Add URL validation to operational server webhooks by @CodeMan62 in #1887
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants