-
-
Notifications
You must be signed in to change notification settings - Fork 713
hotfix: stop debug logs and healthcheck no longer requires prisma #2108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughThe changes across the codebase involve simplifying and disabling several API route handlers and the health check endpoint. In two API routes related to debug log recording, the original implementations—which included parameter validation, database operations, event recording, and detailed error handling—have been commented out and replaced with minimal ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.logs.debug.ts (1)
8-43
: Consistent implementation with the dev route.This follows the same pattern as the dev route handler. The stub implementation is correct for disabling the endpoint.
Consider applying the same improvements suggested for the previous file:
- Remove the commented code if permanent, or add a TODO if temporary
- Update the comment to explain why this endpoint is disabled
🧹 Nitpick comments (4)
apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.logs.debug.ts (3)
1-13
: Clean up unused imports.Most of these imports are no longer used after commenting out the main functionality. Consider removing or commenting out the unused imports to improve code clarity.
-import { TypedResponse } from "@remix-run/server-runtime"; -import { assertExhaustive } from "@trigger.dev/core/utils"; -import { RunId } from "@trigger.dev/core/v3/isomorphic"; -import { - WorkerApiDebugLogBody, - WorkerApiRunAttemptStartResponseBody, -} from "@trigger.dev/core/v3/workers"; -import { z } from "zod"; -import { prisma } from "~/db.server"; -import { logger } from "~/services/logger.server"; -import { createActionApiRoute } from "~/services/routeBuilders/apiBuilder.server"; -import { recordRunDebugLog } from "~/v3/eventRepository.server";
14-74
: Consider removing commented code or adding a TODO comment.Large blocks of commented code can reduce code readability. Since this is labeled as a hotfix:
- If this change is permanent, consider deleting the commented code entirely
- If temporary, add a TODO comment explaining when and why this should be restored
+// TODO: Temporarily disabled debug logging for performance reasons. +// Restore this functionality after [specific condition or date] // const { action } = createActionApiRoute(
76-79
: Add documentation for the stub implementation.The stub implementation correctly returns 204 No Content. Consider adding a comment explaining why this endpoint is disabled to help future developers understand the context.
-// Create a generic JSON action in remix +// Temporarily disabled debug logging endpoint - returns 204 No Content without processing +// See PR #2108 for context export function action() { return new Response(null, { status: 204 }); }apps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.logs.debug.ts (1)
1-6
: Remove unused imports.Similar to the previous file, these imports are no longer used and should be cleaned up.
-import { assertExhaustive } from "@trigger.dev/core/utils"; -import { RunId } from "@trigger.dev/core/v3/isomorphic"; -import { WorkerApiDebugLogBody } from "@trigger.dev/core/v3/runEngineWorker"; -import { z } from "zod"; -import { createActionWorkerApiRoute } from "~/services/routeBuilders/apiBuilder.server"; -import { recordRunDebugLog } from "~/v3/eventRepository.server";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/routes/engine.v1.dev.runs.$runFriendlyId.logs.debug.ts
(1 hunks)apps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.logs.debug.ts
(1 hunks)apps/webapp/app/routes/healthcheck.tsx
(0 hunks)
💤 Files with no reviewable changes (1)
- apps/webapp/app/routes/healthcheck.tsx
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
No description provided.