Conversation
- c2j#126: write access logs in Apache Combined Log Format (IP first, CLF timestamp with timezone, real HTTP version, status, bytes, Referer, User-Agent, latency) using chrono instead of hand-rolled date math - c2j#125: /api/v1/trace with no matching node returns 200 with an empty result object instead of 404 - add chrono as a serve-gated optional dependency - add serve_api integration tests for both behaviors Pre-existing, unrelated failures on this toolchain (verified identical on the base commit): - clippy --features full -- -D warnings: 8 errors in untouched files - cargo test --features full: test_path_mapping_applied fails on Windows (forward-slash path assertion)
c2j
requested changes
Aug 10, 2026
c2j
left a comment
Owner
There was a problem hiding this comment.
PR #129 Review — fix(serve): standardize access log format and trace empty-match
总结:将 access log 改为 Apache Combined Log Format(新增 chrono 可选依赖),并包含 trace 空匹配改动(与 PR#127 重叠),新增集成测试验证行为。
需要作者处理:
- PR 与其它分支(例如 #127)存在重叠改动并显示 CONFLICTING。请解决冲突并去重(保留最终实现),并在 PR 描述说明与 #127 的关系;若两个 PR 意图合并为同一行为,建议合并为单一 PR。
- 依赖变更:新增 chrono(serve 可选依赖),请在 PR 描述或 CHANGELOG 补充说明,并在 CI 中验证 cargo build --features full 成功。
- middleware 使用 response.headers() 等 API 时,请确认没有引入 move/borrow 的错误(建议在 CI 做一次 serve 的端到端 smoke 测试)。
- 测试稳定性:access log 测试写入 .codeweb/http.log,测试前应清理或使用唯一路径,避免竞争或残留影响结果。
- 请确保本 PR 不引入新的 clippy/fmt 问题(运行 cargo fmt -- --check 与 cargo clippy --features serve -- -D warnings 并在 PR 中附上输出)。
结论建议:功能方向合理,待解决冲突、补充说明并保证 CI 干净后合并。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #125, fixes #126.
Changes
serve: HTTP 访问日志格式不标准 #126 — HTTP access log format: rewrite
src/server/access_log.rsto emit the Apache Combined Log Format instead of the ad-hoctimestamp INFO "METHOD URI HTTP/1.1" status ms ipline.Before:
After:
chrono_now()leap-year logic) withchrono::Local.chronoas aserve-gated optional dependency.serve:
trace端点空匹配返回 200 而非 404 #125 — trace empty-match:/api/v1/tracewith no matching node now returns200with an empty result object instead of404:{"target":null,"callers":[],"callees":[],"caller_count":0,"callee_count":0,"truncated":false}Tests: add
test_serve_trace_empty_returns_200andtest_serve_access_log_combined_formatintests/serve_api.rs.Verification
cargo build(default) andcargo build --features fullpass.cargo test --features serve --test serve_api: 9/9 pass.cargo fmt -- --checkpasses.cargo clippy --features full -- -D warningserrors are pre-existing in untouched files.Pre-existing failures (unrelated, verified identical on base)
cargo clippy --features full -- -D warnings: 8 errors in untouched files (src/graph/builder.rs,src/graph/inspect.rs,src/graph/query/traversal.rs,src/mark.rs,src/parser/jsp_loader.rs,src/server/handlers.rs:613,src/main.rs).cargo test --features full:import::parser::tests::test_path_mapping_appliedfails on Windows (asserts forward-slash path/prefix/sql/a.sql).Note: building on Windows GNU target requires
dlltoolon PATH (chrono'swindows-linkbuild script).