Skip to content

Conversation

@shakeeb-upstart
Copy link
Contributor

Created an extension provider for HealthCheckRoutes, so users can get the routes and add it to their own server

@shakeeb-upstart shakeeb-upstart marked this pull request as ready for review October 2, 2024 13:48
@pjfanning
Copy link
Member

@shakeeb-upstart would you be able to add a unit test for regression purposes?

override def routes(mrps: ManagementRouteProviderSettings): Route = routes()

/**
* @since 1.1.0
Copy link
Member

Choose a reason for hiding this comment

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

1.1.0 was already released so we need to decide whether this can go in 1.1.1 or if we need a 1.2.0 before this can be released

Copy link
Member

@pjfanning pjfanning Nov 20, 2024

Choose a reason for hiding this comment

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

not much point changing this yet - but I will block the merge until we can discuss how to eventually release this (which affects which branch we can use)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes sure, let me know if I am needed for any part in the discussion.

Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

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

Would PekkoManagement(sys).routes not work in your case? do you really need explicitly the health check routes without any other Management routes?

* @since 1.1.0
*/
object HealthCheckRoutes extends ExtensionId[HealthCheckRoutes] with ExtensionIdProvider {
override def get(system: ActorSystem): HealthCheckRoutes = super.get(system)
Copy link
Member

Choose a reason for hiding this comment

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

returning it here means HealthCheckRoutes is becoming public API, so we should remove the 'internal' markers.

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