Default Warning logging, bounded container logs, remove Node.js implementation#49
Conversation
…s implementation - Program.cs: default log level Warning unless Logging__LogLevel__Default is set — per-request Information logging was ~290 MiB/day into unbounded docker json-file logs - docker-compose.yml: json-file max-size/max-file bounds as belt-and-suspenders - Remove the Node/Express implementation (src/, build config, root Dockerfile, compose, healthCheck.js, crowdin config): the C# service is live in all regions; the last Node build stays available as ecency/api:node-legacy - parity/driver.py: generate the request catalog from Routes.cs (legacy index.tsx still supported via VAPI_INDEX_TSX for testing old builds) - Rewrite root README for the .NET service (quickstart, env vars, swarm, deploy/rollback); update dotnet/README (vector regen without repo node_modules, node-legacy parity reference, log level)
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe pull request removes the legacy Node/React API implementation and related tooling, updates documentation for the .NET API, adds container logging controls, and changes parity catalog generation to use the C# route table by default. Changes.NET migration and operational documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dotnet/docker-compose.yml`:
- Around line 30-36: Update the service’s explicit environment list in
docker-compose.yml to forward the documented Logging__LogLevel__Default variable
into the container, allowing Swarm deployments to override the default logging
level honored by Program.cs while preserving the existing Warning fallback.
In `@dotnet/README.md`:
- Around line 290-292: Update the documented vector-generation command around
tools/gen-vectors.js so dependency installation occurs in a subshell without
changing the caller’s working directory, then execute the generator from dotnet/
with repository-relative script and fixture paths resolving correctly.
In `@README.md`:
- Around line 34-36: Update the Docker quick-start example’s PRIVATE_API_AUTH
value to a valid base64-encoded JSON example consistent with the environment
table, while leaving the surrounding Docker command and other environment values
unchanged.
- Around line 74-75: Update both rollback commands in the README to target the
deployed Docker Swarm service vision-api_app instead of vision_vapi, while
preserving the existing image tags and rollback descriptions.
- Around line 7-10: Update the parity suite case count in README.md from 300 to
305, matching dotnet/README.md and the latest recorded verification result while
preserving the surrounding migration and drop-in compatibility claims.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5eb04a70-3b45-4e2b-9bc8-06f5a57cca16
⛔ Files ignored due to path filters (5)
dotnet/parity/__pycache__/driver.cpython-312.pycis excluded by!**/*.pycpublic/favicon.icois excluded by!**/*.icopublic/favicon.pngis excluded by!**/*.pngpublic/logo512.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (40)
.dockerignore.gitignoreDockerfileREADME.mdcrowdin.ymldocker-compose.ymldotnet/EcencyApi/Program.csdotnet/README.mddotnet/docker-compose.ymldotnet/parity/driver.pyhealthCheck.jspackage.jsonpublic/robots.txtrazzle.config.jssrc/client/index.tsxsrc/common/app.tsxsrc/common/helper/hive-signer.tssrc/common/util/b64.tssrc/config.tssrc/index.tssrc/models/converters.tssrc/models/hiveEngine.types.tssrc/server/cache.tssrc/server/chain-providers.tssrc/server/handlers/announcements.tssrc/server/handlers/auth-api.tssrc/server/handlers/constants.tssrc/server/handlers/fallback.tsxsrc/server/handlers/hive-explorer.tssrc/server/handlers/private-api.test.tssrc/server/handlers/private-api.tssrc/server/handlers/search-api.tssrc/server/handlers/spotlights.tssrc/server/handlers/wallet-api.tssrc/server/helper.tssrc/server/index.tsxsrc/server/template.tsxsrc/server/util.tstsconfig.jsontslint.json
💤 Files with no reviewable changes (34)
- public/robots.txt
- tslint.json
- healthCheck.js
- package.json
- crowdin.yml
- src/server/handlers/private-api.test.ts
- src/server/handlers/spotlights.ts
- src/common/app.tsx
- src/index.ts
- docker-compose.yml
- src/server/handlers/hive-explorer.ts
- razzle.config.js
- tsconfig.json
- src/models/converters.ts
- src/common/util/b64.ts
- src/server/handlers/announcements.ts
- src/client/index.tsx
- src/server/handlers/constants.ts
- src/common/helper/hive-signer.ts
- src/server/index.tsx
- Dockerfile
- src/server/cache.ts
- src/config.ts
- src/server/template.tsx
- src/server/handlers/auth-api.ts
- src/server/handlers/fallback.tsx
- src/server/handlers/search-api.ts
- src/server/helper.ts
- src/server/handlers/wallet-api.ts
- src/models/hiveEngine.types.ts
- .dockerignore
- src/server/util.ts
- src/server/chain-providers.ts
- src/server/handlers/private-api.ts
| Implemented in **C# / ASP.NET Core (.NET 10)** under [`dotnet/`](dotnet/). It | ||
| replaced the original Node/Express implementation as a verified drop-in | ||
| (byte-identical responses across a 300-case differential parity suite; the | ||
| history of that migration is in the git log and `dotnet/README.md`). The last |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the documented parity case count.
This says the suite has 300 cases, while dotnet/README.md and the latest recorded result state 305 cases. Keep the verification claims consistent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 7 - 10, Update the parity suite case count in
README.md from 300 to 305, matching dotnet/README.md and the latest recorded
verification result while preserving the surrounding migration and drop-in
compatibility claims.
| docker service update --image ecency/api:sha-<previous-commit> vision_vapi | ||
| docker service update --image ecency/api:node-legacy vision_vapi # pre-rewrite Node build |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fix the rollback service target.
The stack is deployed as vision-api with an app service, so Docker Swarm will create vision-api_app; neither rollback command targets the service declared by dotnet/docker-compose.yml (vision_vapi). These commands will fail unless another stack defines that service.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 74 - 75, Update both rollback commands in the README
to target the deployed Docker Swarm service vision-api_app instead of
vision_vapi, while preserving the existing image tags and rollback descriptions.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b09f7ca68b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cd dotnet | ||
| docker stack deploy -c docker-compose.yml vision-api |
There was a problem hiding this comment.
In the Swarm path documented here, docker stack deploy does not build the local dotnet/Dockerfile; Docker's stack-deploy docs show it reports Ignoring unsupported options: build and then deploys the service image from the compose file. The referenced dotnet/docker-compose.yml still names ecency/api-csharp:latest, while CI in .github/workflows/main.yml publishes ecency/api:latest and ecency/api:sha-*, so operators following these new instructions can deploy a stale/wrong image or fail to pull instead of deploying the current build.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR finishes the move from the legacy Node service to the .NET API. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "docs/config: review fixes — service nami..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ules - compose: service renamed app->vapi (stack deploy as 'vision' yields the vision_vapi service the docs reference), image ecency/api:latest (what CI publishes), forward Logging__LogLevel__Default - README: vision-next link, valid base64 PRIVATE_API_AUTH example, consistent 301-case parity count, stack-deploy naming aligned with rollback commands - dotnet/README: vector-regen instructions no longer cd away from the paths they use - .gitignore: restore .env*/node_modules/coverage/build guards
|
All review findings addressed in 4dad539 — this round every one was valid:
Also fixed the footer link to point at ecency/vision-next. 42 tests green; catalog verified to still parse at 301 cases. |
1 participant
Post-cutover housekeeping, verified before push (42 tests green; Warning default confirmed to produce zero per-request log lines while startup lines and warnings/errors remain).
Logging
Logging__LogLevel__Defaultis set. The per-request Information logging wrote ~4 lines per request into unbounded docker json-file logs on the origin hosts.dotnet/docker-compose.ymlbounds container logs (max-size: 20m,max-file: 3) as belt-and-suspenders; the same bounds have been applied to the running services.Node.js removal
The C# service is live in all regions, so the legacy implementation is removed:
src/, build config (razzle/tsconfig/tslint/package.json/yarn.lock), rootDockerfile,docker-compose.yml,healthCheck.js, and the crowdin config. Rollback to the last Node build remains available via theecency/api:node-legacyimage tag — no source needed.The parity harness now generates its request catalog from
EcencyApi/Handlers/Routes.cs; a legacy Express route table is still supported viaVAPI_INDEX_TSXfor testing old images.Docs
README.mdrewritten for the .NET service: quickstart (SDK or Docker), full env-var table, swarm deploy, deployment/rollback with the tag scheme.dotnet/README.mdupdated: regenerating crypto vectors without repo-local node_modules, usingnode-legacyas the parity reference (with the restart-policy caveat, since the legacy build crashes on the malformed hs-token-refresh probe), and the new logging default.Summary by CodeRabbit
New Features
Bug Fixes
Chores