Add keep-alive handlers and database ping test #98
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRenames Ping to KeepAlive across handlers, payloads, tests, and routing. Adds a BasicAuth-protected /ping-db route that pings the database. Introduces a no-cache HTTP response helper. Changes database Connection.Ping to return error and updates call sites. Adjusts app boot and router wiring accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Router
participant KeepAliveHandler as KeepAliveHandler
participant HTTP as http.Response
Client->>Router: GET /ping
Router->>KeepAliveHandler: Handle(w, r)
KeepAliveHandler->>HTTP: MakeNoCacheResponse(w, r)
KeepAliveHandler-->>Client: 200 OK {message:"pong", dateTime:"..."}
%% Note: No auth, no DB
note over KeepAliveHandler: Constructs payload.KeepAliveResponse
sequenceDiagram
autonumber
actor Client
participant Router
participant KeepAliveDB as KeepAliveDBHandler
participant Env as env.Ping
participant DB as database.Connection
participant HTTP as http.Response
Client->>Router: GET /ping-db (BasicAuth)
Router->>KeepAliveDB: Handle(w, r)
KeepAliveDB->>Env: Validate creds
alt Invalid or missing creds
KeepAliveDB-->>Client: 401 Unauthorized
else Valid creds
KeepAliveDB->>DB: Ping()
alt DB ping error
KeepAliveDB-->>Client: 500 Internal Server Error
else OK
KeepAliveDB->>HTTP: MakeNoCacheResponse(w, r)
KeepAliveDB-->>Client: 200 OK {message:"pong", dateTime:"..."}
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @gocanto, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request refactors the existing health check endpoint by renaming it to keep-alive for clearer semantics and introduces a dedicated health check endpoint for the database connection. These changes improve the application's ability to monitor its operational status and dependencies.
Highlights
- Handler Renaming: The existing generic "ping" handler and its associated files have been renamed to "keep-alive" to better reflect its purpose as a general health check.
- Database Keep-Alive Handler: A new handler, KeepAliveDBHandler, has been introduced to specifically perform a ping test against the database connection, enhancing the application's health monitoring capabilities.
- New Test Coverage: Comprehensive unit tests have been added for the newly introduced KeepAliveDBHandler to ensure its correct functionality and robustness.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request refactors the ping handler to keep-alive, adds a new keep-alive handler to check database connectivity, and includes corresponding tests. The changes are generally good, but there are a few important issues. The new database keep-alive handler doesn't correctly handle database ping failures, which is a critical flaw. There's also some code duplication between the two keep-alive handlers that could be refactored. Finally, the tests for the new database handler are missing a case for when the database connection fails. My review includes suggestions to address these points.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
metal/kernel/router.go (1)
83-91: Register or remove DB keep-alive handler
MakeKeepAliveDBHandler is never wired in metal/kernel/router.go (no GET /ping/db). Either expose it in Router.Ping or remove the unused handler.handler/keep_alive.go (1)
22-45: Avoid caching authenticated keep-alive responses.
http.MakeResponseFromsetsCache-Control: public, max-age=3600. With BasicAuth,publicenables shared caches to store responses (RFC 7234). For an auth-gated health endpoint, preferno-store.Two approaches:
- Add a no-cache variant in pkg/http and use it here:
// pkg/http: add a constructor that sets Cache-Control: no-store func MakeNoCacheResponse(writer baseHttp.ResponseWriter, request *baseHttp.Request) *Response { ... }Then:
- resp := http.MakeResponseFrom("0.0.1", w, r) + resp := http.MakeNoCacheResponse(w, r)
- Or extend
MakeResponseFromwith options to override cache headers for auth endpoints.
🧹 Nitpick comments (7)
handler/payload/keep_alive.go (1)
3-7: Consider clarifying time semantics.If this is always UTC, add it to the doc comment to avoid client ambiguity.
metal/kernel/router.go (2)
83-91: Use the existing public pipeline helper for consistency.You can simplify and ensure common public middleware by using
PublicPipelineFor.func (r *Router) Ping() { - abstract := handler.MakeKeepAliveHandler(&r.Env.Ping) - - apiHandler := http.MakeApiHandler( - r.Pipeline.Chain(abstract.Handle), - ) - - r.Mux.HandleFunc("GET /ping", apiHandler) + abstract := handler.MakeKeepAliveHandler(&r.Env.Ping) + apiHandler := r.PublicPipelineFor(abstract.Handle) + r.Mux.HandleFunc("GET /ping", apiHandler) }
83-91: Endpoint name vs. function name.Method remains
Ping()though the handler is “keep-alive.” Optional rename toKeepAlive()for clarity.handler/keep_alive_test.go (2)
15-18: Credential length may conflict with future validation.
env.Pinghasmin=16tags. Tests use short creds; if validation is later enforced, tests will break. Safer to use 16+ chars.- e := env.Ping{Username: "user", Password: "pass"} + e := env.Ping{Username: "0123456789abcdef", Password: "fedcba9876543210"}And update
SetBasicAuthbelow accordingly.
15-49: Optional: mark as parallel.This unit test doesn’t use shared resources;
t.Parallel()can speed up the suite.handler/keep_alive_db_test.go (1)
16-20: Use long credentials to align with struct tags.Future validation could enforce
min=16.- e := env.Ping{Username: "user", Password: "pass"} + e := env.Ping{Username: "0123456789abcdef", Password: "fedcba9876543210"}And update the
SetBasicAuthcalls below.handler/keep_alive.go (1)
32-38: Version salt should come from a single source.Hardcoding
"0.0.1"risks drift. Consider a build-time var or shared const.- resp := http.MakeResponseFrom("0.0.1", w, r) + resp := http.MakeResponseFrom(portal.Version, w, r) // or env/build var
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
handler/keep_alive.go(2 hunks)handler/keep_alive_db.go(1 hunks)handler/keep_alive_db_test.go(1 hunks)handler/keep_alive_test.go(2 hunks)handler/payload/keep_alive.go(1 hunks)handler/payload/ping.go(0 hunks)metal/kernel/router.go(1 hunks)
💤 Files with no reviewable changes (1)
- handler/payload/ping.go
🧰 Additional context used
🧬 Code graph analysis (6)
handler/keep_alive_db_test.go (4)
handler/tests/db.go (1)
MakeTestDB(18-88)metal/env/ping.go (1)
Ping(5-8)handler/payload/keep_alive.go (1)
KeepAliveResponse(4-7)pkg/portal/consts.go (1)
DatesLayout(3-3)
handler/payload/keep_alive.go (1)
pkg/http/schema.go (1)
Message(11-15)
handler/keep_alive_db.go (6)
metal/env/ping.go (1)
Ping(5-8)database/connection.go (1)
Connection(13-18)pkg/http/schema.go (1)
ApiError(11-15)pkg/http/response.go (3)
LogUnauthorisedError(101-108)MakeResponseFrom(19-39)LogInternalError(76-83)handler/payload/keep_alive.go (1)
KeepAliveResponse(4-7)pkg/portal/consts.go (1)
DatesLayout(3-3)
handler/keep_alive_test.go (3)
metal/env/ping.go (1)
Ping(5-8)handler/keep_alive.go (1)
MakeKeepAliveHandler(18-20)handler/payload/keep_alive.go (1)
KeepAliveResponse(4-7)
metal/kernel/router.go (2)
handler/keep_alive.go (1)
MakeKeepAliveHandler(18-20)metal/env/ping.go (1)
Ping(5-8)
handler/keep_alive.go (5)
metal/env/ping.go (1)
Ping(5-8)pkg/http/schema.go (1)
ApiError(11-15)handler/payload/keep_alive.go (1)
KeepAliveResponse(4-7)pkg/portal/consts.go (1)
DatesLayout(3-3)pkg/http/response.go (1)
LogInternalError(76-83)
🔇 Additional comments (5)
handler/payload/keep_alive.go (1)
3-7: LGTM: Simple, clear DTO.Structure and tags look good.
handler/keep_alive_test.go (1)
19-39: Test asserts are solid.Covers status, payload, and datetime format. Nice.
handler/keep_alive_db_test.go (1)
43-51: Auth negative-path assertion is correct.Unauthorized flow is properly validated.
handler/keep_alive.go (1)
22-45: Core flow looks good.Auth check, UTC timestamp, and payload encoding are straightforward and correct.
handler/keep_alive_db.go (1)
20-22: Value vs. pointer receiver is fine here.Small struct; returning by value is okay.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
handler/keep_alive.go (1)
22-30: Use constant-time comparison in HasInvalidCreds
In metal/env/ping.go, HasInvalidCreds currently doesreturn username != strings.TrimSpace(p.Username) || password != strings.TrimSpace(p.Password)Replace these plain
!=checks withcrypto/subtle.ConstantTimeCompareon the trimmed stored values versus the provided inputs to eliminate timing side-channels (and addimport "crypto/subtle").
♻️ Duplicate comments (1)
handler/keep_alive_db_test.go (1)
52-61: Failure-path test added — previous feedback addressed.The 500-on-ping-failure path is now covered. Thanks for closing the gap.
🧹 Nitpick comments (12)
pkg/http/response_test.go (1)
34-59: Solid no-cache coverage; consider asserting more headers.Looks good. For completeness, also assert Content-Type and X-Content-Type-Options set by MakeNoCacheResponse.
- if rec.Code != http.StatusOK { + if rec.Code != http.StatusOK { t.Fatalf("status %d", rec.Code) } if rec.Header().Get("Cache-Control") != "no-store" { t.Fatalf("unexpected cache-control: %s", rec.Header().Get("Cache-Control")) } if rec.Header().Get("ETag") != "" { t.Fatalf("etag should be empty") } +if rec.Header().Get("Content-Type") != "application/json" { + t.Fatalf("content-type mismatch: %s", rec.Header().Get("Content-Type")) +} +if rec.Header().Get("X-Content-Type-Options") != "nosniff" { + t.Fatalf("x-content-type-options mismatch: %s", rec.Header().Get("X-Content-Type-Options")) +}main.go (1)
38-40: DB ping on startup: OK to log-only?Confirm that continuing startup on ping failure is intended. If not, fail fast or expose a health endpoint for liveness/readiness.
Also, drop the “Testing” comment to avoid confusion in prod logs.
- // --- Testing if err := app.GetDB().Ping(); err != nil { slog.Error("database ping failed", "error", err) }Optionally, add a timeout in Connection.Ping() (see suggestion in database/connection.go).
metal/kernel/router_keep_alive_test.go (1)
12-39: Test credentials below validation min lengths; use longer strings to prevent future breakage.env.Ping enforces min=16; if validation is later applied here, this could fail. Also consider asserting no-cache headers on responses.
- r := Router{ - Env: &env.Environment{Ping: env.Ping{Username: "user", Password: "pass"}}, + r := Router{ + Env: &env.Environment{Ping: env.Ping{Username: "user-cred-should-be-16", Password: "pass-cred-should-be-16"}},Optional header assertions:
rec := httptest.NewRecorder() r.Mux.ServeHTTP(rec, req) if rec.Code != http.StatusOK { t.Fatalf("expected status %d, got %d", http.StatusOK, rec.Code) } +if h := rec.Header().Get("Cache-Control"); h != "no-store" { + t.Fatalf("cache-control: %s", h) +} +if ct := rec.Header().Get("Content-Type"); ct != "application/json" { + t.Fatalf("content-type: %s", ct) +}pkg/http/response.go (1)
41-54: MakeNoCacheResponse: add legacy no-cache headers for intermediaries.Add Pragma and Expires for broader proxy behavior; harmless alongside no-store.
func MakeNoCacheResponse(writer baseHttp.ResponseWriter, request *baseHttp.Request) *Response { cacheControl := "no-store" return &Response{ writer: writer, request: request, cacheControl: cacheControl, headers: func(w baseHttp.ResponseWriter) { w.Header().Set("Content-Type", "application/json") w.Header().Set("X-Content-Type-Options", "nosniff") w.Header().Set("Cache-Control", cacheControl) + w.Header().Set("Pragma", "no-cache") + w.Header().Set("Expires", "0") }, } }database/connection.go (1)
49-60: Use PingContext with timeout to avoid startup hangs.A stuck DNS/TCP can hang Ping() indefinitely. Add a short timeout.
func (c *Connection) Ping() error { conn, err := c.driver.DB() if err != nil { return fmt.Errorf("error retrieving the db driver: %w", err) } - if err := conn.Ping(); err != nil { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + if err := conn.PingContext(ctx); err != nil { return fmt.Errorf("error pinging the db driver: %w", err) } return nil }Add imports:
// at top of file import ( "context" "time" )metal/kernel/router_keep_alive_db_test.go (1)
13-42: Good route coverage; add a ping-failure case and align creds with validation constraints.
- Use 16+ char creds per env.Ping tags.
- Add subtest that closes DB before request to assert 500.
- r := Router{ - Env: &env.Environment{Ping: env.Ping{Username: "user", Password: "pass"}}, + r := Router{ + Env: &env.Environment{Ping: env.Ping{Username: "user-cred-should-be-16", Password: "pass-cred-should-be-16"}}, Db: db, Mux: http.NewServeMux(), Pipeline: middleware.Pipeline{PublicMiddleware: middleware.MakePublicMiddleware("", false)}, } r.KeepAliveDB() @@ }) + t.Run("db ping failure", func(t *testing.T) { + // Force failure by closing the DB before the request. + if ok := db.Close(); !ok { + t.Fatalf("failed to close db for test") + } + req := httptest.NewRequest("GET", "/ping-db", nil) + req.SetBasicAuth("user-cred-should-be-16", "pass-cred-should-be-16") + rec := httptest.NewRecorder() + r.Mux.ServeHTTP(rec, req) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected status %d, got %d", http.StatusInternalServerError, rec.Code) + } + })metal/kernel/router.go (1)
83-101: Route wiring: prefer PublicPipelineFor for consistency and middleware coverage.Other public routes use PublicMiddleware (rate-limit, TTL, IP rules). KeepAlive endpoints should likely do the same.
func (r *Router) KeepAlive() { - abstract := handler.MakeKeepAliveHandler(&r.Env.Ping) - - apiHandler := http.MakeApiHandler( - r.Pipeline.Chain(abstract.Handle), - ) - - r.Mux.HandleFunc("GET /ping", apiHandler) + abstract := handler.MakeKeepAliveHandler(&r.Env.Ping) + r.Mux.HandleFunc("GET /ping", r.PublicPipelineFor(abstract.Handle)) } func (r *Router) KeepAliveDB() { - abstract := handler.MakeKeepAliveDBHandler(&r.Env.Ping, r.Db) - - apiHandler := http.MakeApiHandler( - r.Pipeline.Chain(abstract.Handle), - ) - - r.Mux.HandleFunc("GET /ping-db", apiHandler) + abstract := handler.MakeKeepAliveDBHandler(&r.Env.Ping, r.Db) + r.Mux.HandleFunc("GET /ping-db", r.PublicPipelineFor(abstract.Handle)) }handler/keep_alive_db_test.go (3)
16-20: Use 16+ char credentials to match env.Ping validation and future-proof the tests.env.Ping enforces min=16 via tags; aligning tests reduces churn if validation is applied in construction paths.
- e := env.Ping{Username: "user", Password: "pass"} + e := env.Ping{Username: "0123456789abcdef", Password: "fedcba9876543210"}And update BasicAuth calls:
- req.SetBasicAuth("user", "pass") + req.SetBasicAuth("0123456789abcdef", "fedcba9876543210")- req.SetBasicAuth("user", "pass") + req.SetBasicAuth("0123456789abcdef", "fedcba9876543210")
21-41: Assert no-cache and content-type headers on success.KeepAlive uses MakeNoCacheResponse; validating headers catches regressions.
if rec.Code != http.StatusOK { t.Fatalf("status %d", rec.Code) } + if got := rec.Header().Get("Cache-Control"); got != "no-store" { + t.Fatalf("unexpected Cache-Control: %q", got) + } + if ct := rec.Header().Get("Content-Type"); ct != "application/json" { + t.Fatalf("unexpected Content-Type: %q", ct) + }
52-61: Isolate the failure-path by not mutating the shared DB used by other subtests.Use a fresh connection for the failure case to avoid side effects.
- t.Run("db ping failure", func(t *testing.T) { - db.Close() - req := httptest.NewRequest("GET", "/ping-db", nil) - req.SetBasicAuth("user", "pass") - rec := httptest.NewRecorder() - if err := h.Handle(rec, req); err == nil || err.Status != http.StatusInternalServerError { - t.Fatalf("expected internal error, got %#v", err) - } - }) + t.Run("db ping failure", func(t *testing.T) { + db2, _ := handlertests.MakeTestDB(t) + h2 := MakeKeepAliveDBHandler(&e, db2) + _ = db2.Close() // simulate DB down, keep other subtests unaffected + req := httptest.NewRequest("GET", "/ping-db", nil) + req.SetBasicAuth("0123456789abcdef", "fedcba9876543210") + rec := httptest.NewRecorder() + if err := h2.Handle(rec, req); err == nil || err.Status != http.StatusInternalServerError { + t.Fatalf("expected internal error, got %#v", err) + } + })handler/keep_alive.go (2)
22-30: Minor cleanup: prefer errors.New over fmt.Errorf with a constant string.-import ( - "fmt" +import ( + "errors" baseHttp "net/http" "time" ) @@ - fmt.Errorf("invalid credentials"), + errors.New("invalid credentials"),
22-30: Set WWW-Authenticate header on 401 for BasicAuth clients.Browsers and other HTTP clients require this header to trigger a credentials prompt. Add immediately before returning the unauthorized error:
func (h KeepAliveHandler) Handle(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { user, pass, ok := r.BasicAuth() if !ok || h.env.HasInvalidCreds(user, pass) { + w.Header().Set("WWW-Authenticate", `Basic realm="keep-alive", charset="UTF-8"`) return http.LogUnauthorisedError( "invalid credentials", fmt.Errorf("invalid credentials"), ) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
database/connection.go(1 hunks)handler/keep_alive.go(2 hunks)handler/keep_alive_db.go(1 hunks)handler/keep_alive_db_test.go(1 hunks)handler/payload/keep_alive.go(1 hunks)main.go(1 hunks)metal/kernel/app.go(1 hunks)metal/kernel/router.go(2 hunks)metal/kernel/router_keep_alive_db_test.go(1 hunks)metal/kernel/router_keep_alive_test.go(1 hunks)pkg/http/response.go(2 hunks)pkg/http/response_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- handler/payload/keep_alive.go
- handler/keep_alive_db.go
🧰 Additional context used
🧬 Code graph analysis (9)
main.go (1)
metal/kernel/helpers.go (1)
a(22-28)
metal/kernel/router_keep_alive_test.go (5)
metal/kernel/router.go (1)
Router(25-31)metal/env/ping.go (1)
Ping(5-8)pkg/middleware/pipeline.go (1)
Pipeline(10-15)pkg/middleware/public_middleware.go (2)
PublicMiddleware(20-29)MakePublicMiddleware(31-42)metal/kernel/router_signature_test.go (1)
TestSignatureRoute_PublicMiddleware(15-65)
metal/kernel/router_keep_alive_db_test.go (5)
handler/tests/db.go (1)
MakeTestDB(18-88)metal/kernel/router.go (1)
Router(25-31)metal/env/env.go (1)
Environment(9-16)metal/env/ping.go (1)
Ping(5-8)pkg/middleware/public_middleware.go (2)
PublicMiddleware(20-29)MakePublicMiddleware(31-42)
pkg/http/response_test.go (1)
pkg/http/response.go (1)
MakeNoCacheResponse(41-54)
metal/kernel/router.go (5)
handler/keep_alive.go (1)
MakeKeepAliveHandler(18-20)metal/env/ping.go (1)
Ping(5-8)handler/keep_alive_db.go (1)
MakeKeepAliveDBHandler(20-22)pkg/http/handler.go (1)
MakeApiHandler(9-28)pkg/middleware/pipeline.go (1)
Pipeline(10-15)
handler/keep_alive_db_test.go (5)
handler/tests/db.go (1)
MakeTestDB(18-88)metal/env/ping.go (1)
Ping(5-8)handler/keep_alive_db.go (1)
MakeKeepAliveDBHandler(20-22)handler/payload/keep_alive.go (1)
KeepAliveResponse(3-6)pkg/portal/consts.go (1)
DatesLayout(3-3)
metal/kernel/app.go (1)
metal/kernel/kernel_test.go (1)
TestAppBootRoutes(138-197)
database/connection.go (2)
database/seeder/seeds/seeder_test.go (1)
testConnection(16-88)database/connection_testhelpers.go (1)
NewConnectionFromGorm(6-8)
handler/keep_alive.go (5)
metal/env/ping.go (1)
Ping(5-8)pkg/http/schema.go (1)
ApiError(11-15)pkg/http/response.go (2)
MakeNoCacheResponse(41-54)LogInternalError(95-102)handler/payload/keep_alive.go (1)
KeepAliveResponse(3-6)pkg/portal/consts.go (1)
DatesLayout(3-3)
🔇 Additional comments (5)
metal/kernel/app.go (1)
72-74: Boot now wires KeepAlive and KeepAliveDB—LGTM.Matches PR intent; routes are initialized early in boot.
pkg/http/response.go (1)
71-74: HasCache short-circuit on empty ETag—LGTM.Prevents false positives for no-cache responses.
handler/keep_alive.go (3)
14-20: Rename and constructor look good.Type and factory rename are consistent with the PR’s intent.
32-38: LGTM: no-cache response and payload formatting.Using MakeNoCacheResponse and portal.DatesLayout is consistent with the new policy.
40-41: LGTM: internal error message clarifies the failure point.
gocanto
deleted the
codex/rename-ping.go-to-keep_alive.go-and-add-keep_alive_db.go
branch
Summary
Testing
go test ./handler -run KeepAliveDB -count=1 -vgo test ./metal/kernel -run Ping -count=1 -vhttps://chatgpt.com/codex/tasks/task_e_68c0d3ec39ac83338f20efabf010fdc9
Summary by CodeRabbit
New Features
Improvements
Tests