Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #18453 +/- ##
===========================================
+ Coverage 75.31% 80.21% +4.90%
===========================================
Files 187 132 -55
Lines 11199 7168 -4031
===========================================
- Hits 8434 5750 -2684
+ Misses 2621 1418 -1203
+ Partials 144 0 -144
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
So this is only for health checking a kona rollup-boost, not for healthchecking kona as pure CL? If its pure CL it should fit into the existing heathchecks, since kona and op-node should have 1:1 API specs. Can the kona /healthz be modified to exclude the version? We could also modify conductor to not append /healthz automatically to the rollup-boost endpoint. I think that would help in this case and reduce alot of duplicated code, that way we could configure conductor to point anywhere. Ex. I would rather lean the endpoint modifiable, and modify kona to to respond with rollup-boost health somewhere else than implement a whole new client |
|
Closing in favor of op-rs/kona#3130 |
|
Reopening in combination with op-rs/kona#3131 |
a82c82e to
9c2643c
Compare
b2e2f10 to
a22e8a5
Compare
e3ca48d to
3d5cd48
Compare
-next suffix
3d5cd48 to
04a6966
Compare
jelias2
left a comment
There was a problem hiding this comment.
func (c *RollupBoostClient) Healthcheck(ctx context.Context) (HealthStatus, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.url, nil)
if err != nil {
return "", fmt.Errorf("failed to create request: %w", err)
}
resp, err := c.httpClient.Do(req)
if err != nil {
return "", fmt.Errorf("failed to make request: %w", err)
}
defer resp.Body.Close()
// Toggle behavior here
if c.useJSON {
return c.parseJSONResponse(resp)
} else {
return c.parseStatusCodeResponse(resp)
}
}
```
I'm curious can this PR just be boiled down to something like above?
4ad33eb to
48400ea
Compare
I can see how that's DRYer but I think it's outweighed by sticking the idiom of keeping clients and protocols 1:1. Happy to discuss however. |
48400ea to
2ddf42b
Compare
2ddf42b to
499e77e
Compare
…g to legacy Introduce a new rollup-boost healthcheck that parses JSON responses from the /healthz endpoint. The existing HTTP status code-based healthcheck is renamed to "legacy" and remains available for backward compatibility. The two healthcheck modes are mutually exclusive.
499e77e to
85c3428
Compare
Adds a new JSON-based rollup-boost healthcheck alongside the existing HTTP status code-based implementation.
Standard healthcheck (
--rollup-boost.enabled):execution.rpcwith/healthzappended automaticallyNext healthcheck (
--rollup-boost.next-enabled):--rollup-boost.next-healthcheck-urlThe two modes are mutually exclusive. Both use the same
--rollup-boost.healthcheck-timeoutsetting.CLI/ENV Changes
--rollup-boost.enabled--rollup-boost.next-enabled--rollup-boost.healthcheck-timeout(shared)--rollup-boost.next-healthcheck-urlOP_CONDUCTOR_ROLLUP_BOOST_ENABLEDOP_CONDUCTOR_ROLLUP_BOOST_NEXT_ENABLEDOP_CONDUCTOR_ROLLUP_BOOST_HEALTHCHECK_TIMEOUT(shared)OP_CONDUCTOR_ROLLUP_BOOST_NEXT_HEALTHCHECK_URLBreaking Changes
None. Existing users of
--rollup-boost.enabledare unaffected.