Queues page and pausing the environment/queues #1805
Conversation
# Conflicts: # apps/webapp/app/components/SetupCommands.tsx # apps/webapp/app/components/navigation/SideMenu.tsx
|
WalkthroughThis pull request introduces widespread changes across configuration files, components, routes, services, and database schemas. Updates include modifications to ignore settings and IDE configurations, removal and refactoring of unused imports, new React components and UI elements for queue and environment management, enhancements to routing and presenter logic, and added services for pausing/resuming queues and environments. Database schemas and run-engine modules have been updated to support new paused states, and the public API has been expanded in the core and SDK packages. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Route
participant PauseService
participant Database
Client->>API_Route: Sends pause/resume request for queue/environment
API_Route->>PauseService: Invoke pause/resume service method with action, environment/queue ID
PauseService->>Database: Update paused status & adjust concurrency limits
Database-->>PauseService: Return update result
PauseService-->>API_Route: Return success/failure response
API_Route-->>Client: JSON response with updated status
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (21)
.gitignore (1)
61-62: New GitIgnore Entries for .husky and react-hooks package.jsonThe addition of
.huskyand/packages/react-hooks/src/package.jsonensures that auto-generated or now redundant configuration files are not tracked by Git. Verify that ignoring these paths aligns with your CI/CD and local development workflows, especially if any hooks or package-specific configurations are expected in certain environments.references/hello-world/src/trigger/queues.ts (6)
3-41: Add error handling to queue operationsThe
queuesTestertask performs multiple asynchronous queue operations without any error handling. Consider adding try/catch blocks to properly handle potential failures when listing, retrieving, pausing, or resuming queues.export const queuesTester = task({ id: "queues-tester", run: async (payload: any, { ctx }) => { + try { const q = await queues.list(); for await (const queue of q) { logger.log("Queue", { queue }); } const retrievedFromId = await queues.retrieve(ctx.queue.id); logger.log("Retrieved from ID", { retrievedFromId }); const retrievedFromCtxName = await queues.retrieve({ type: "task", name: ctx.queue.name, }); logger.log("Retrieved from name", { retrievedFromCtxName }); //pause the queue const pausedQueue = await queues.pause({ type: "task", name: "queues-tester", }); logger.log("Paused queue", { pausedQueue }); const retrievedFromName = await queues.retrieve({ type: "task", name: "queues-tester", }); logger.log("Retrieved from name", { retrievedFromName }); //resume the queue const resumedQueue = await queues.resume({ type: "task", name: "queues-tester", }); logger.log("Resumed queue", { resumedQueue }); + } catch (error) { + logger.error("Queue operations failed", { error }); + } }, });
22-25: Use dynamic queue name from context instead of hardcoded valueThe task hardcodes the queue name "queues-tester" for the pause operation, which could make the code less flexible. Consider using the queue name from the context (
ctx.queue.name) for consistency with the retrieval operations.//pause the queue const pausedQueue = await queues.pause({ type: "task", - name: "queues-tester", + name: ctx.queue.name, });
35-38: Use dynamic queue name from context instead of hardcoded valueSimilar to the pause operation, the resume operation also uses a hardcoded queue name. Consider using the context value for consistency.
//resume the queue const resumedQueue = await queues.resume({ type: "task", - name: "queues-tester", + name: ctx.queue.name, });
3-41: Add JSDoc comments for better documentationConsider adding JSDoc comments to describe the purpose and usage of the
queuesTestertask, which would improve code documentation for other developers.+/** + * Demonstrates the usage of queue-related operations. + * Lists all queues, retrieves queues by ID and name, pauses and resumes a queue. + * Useful for testing and understanding the queue API functionality. + */ export const queuesTester = task({ id: "queues-tester", run: async (payload: any, { ctx }) => { // Rest of the code... }, });
43-52: Add JSDoc comments for otherQueueTaskSimilar to the previous suggestion, consider adding JSDoc comments to document the purpose of this task and its custom queue configuration.
+/** + * Example task demonstrating a custom queue configuration. + * Uses a dedicated queue with limited concurrency. + */ export const otherQueueTask = task({ id: "other-queue-task", queue: { name: "my-custom-queue", concurrencyLimit: 1, }, run: async (payload: any, { ctx }) => { logger.log("Other queue task", { payload }); }, });
1-52: Consider adding examples for direct queue ID usageThe code shows examples of retrieving a queue by ID from context and by name, but doesn't demonstrate directly specifying a queue ID string for the retrieve/pause/resume operations, which might be useful to show as an example.
// Example to add showing direct queue ID specification const queueId = "queue_123456"; // Example queue ID const retrievedDirectly = await queues.retrieve(queueId); logger.log("Retrieved directly by ID", { retrievedDirectly });apps/webapp/app/routes/api.v1.queues.$queueParam.ts (1)
28-44: Good error handling with appropriate status codesThe error handling differentiates between "not found" (404) and other errors (400), providing clear feedback to API consumers. The successful response correctly returns the queue data.
Consider adding more specific error messages in the error response to help troubleshoot API issues.
- return json({ error: result.code }, { status: 404 }); + return json({ error: result.code, message: "Queue not found" }, { status: 404 }); - return json({ error: result.code }, { status: 400 }); + return json({ error: result.code, message: `Failed to retrieve queue: ${result.code}` }, { status: 400 });apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues.stream.tsx (1)
33-53: Basic SSE implementation that could be enhanced with actual queue data.While the current implementation establishes the SSE connection and sends periodic updates, it only sends timestamps without actual queue data. Consider enhancing this to include real-time queue status information such as paused status, queue lengths, or processing metrics.
iterator: async ({ send }) => { + const queues = await $replica.taskQueue.findMany({ + where: { + runtimeEnvironmentId: environment.id + }, + select: { + id: true, + name: true, + paused: true, + concurrencyLimit: true + } + }); + send({ event: "update", - data: new Date().toISOString(), + data: JSON.stringify({ + timestamp: new Date().toISOString(), + environment: { + paused: environment.paused + }, + queues + }), }); },apps/webapp/app/routes/api.v1.queues.ts (1)
13-17: Verify the dummy resource functionThe
findResourcefunction is implemented as a dummy function returning1. While this works for this endpoint since no specific resource is needed, consider documenting why this approach was taken to improve maintainability.- findResource: async () => 1, // This is a dummy function, we don't need to find a resource + findResource: async () => 1, // Dummy resource function returning a non-null value to satisfy the API route builder. + // No specific resource is needed for this endpoint as we're only validating the authentication.apps/webapp/app/components/primitives/PageHeader.tsx (1)
28-32: Good conditional rendering logic for bannersThe updated rendering logic now shows either the
UpgradePromptor theEnvironmentPausedBannerbased on conditions, which is an improvement over the previous implementation.However, the current implementation has a potential issue: the
EnvironmentPausedBannerwill always render when the upgrade prompt is not showing, even if the environment is not paused. According to theEnvironmentPausedBannercomponent's implementation, it does handle this internally by only rendering content whenenvironment.pausedis true, but the component is still being mounted unnecessarily.Consider simplifying the logic to only render components when needed:
- {showUpgradePrompt.shouldShow && organization ? ( - <UpgradePrompt /> - ) : ( - <EnvironmentPausedBanner /> - )} + {showUpgradePrompt.shouldShow && organization && <UpgradePrompt />} + <EnvironmentPausedBanner />This approach allows both components to manage their own visibility based on their internal logic, making the code more maintainable.
apps/webapp/app/env.server.ts (1)
663-664: Consider adding documentation for the new environment variables.The new queue SSE autoreload environment variables are added correctly, but their purpose and impact on the system are not documented with comments, which would improve maintainability.
+ // How frequently (in ms) the queue SSE endpoint should automatically reload data QUEUE_SSE_AUTORELOAD_INTERVAL_MS: z.coerce.number().int().default(5_000), + // Maximum time (in ms) before the queue SSE connection times out QUEUE_SSE_AUTORELOAD_TIMEOUT_MS: z.coerce.number().int().default(60_000),internal-packages/run-engine/src/engine/index.ts (1)
1655-1657: Consider verifying paused state when retrieving concurrency.This method straightforwardly retrieves and returns the concurrency for an environment, but if the environment is paused, it could be worthwhile to return zero or otherwise indicate that concurrency might be zeroed out. Consider adding a check for a paused environment if that state is relevant to concurrency.
apps/webapp/app/presenters/v3/EnvironmentQueuePresenter.server.ts (1)
6-10: Add possibility for additional environment states.Currently, the
Environmentinterface tracksrunning,queued, andconcurrencyLimit. If you anticipate environment pausing or partial availability, consider extending this type to reflect that state (e.g., apausedflag).apps/webapp/app/v3/services/pauseEnvironment.server.ts (1)
25-67: Robust pausing logic with concurrency updates.
- Properly updates the environment
pausedstate in the database.- Logs valuable debug information.
- Resets concurrency to zero on pause, restores concurrency on resume.
- Falls back gracefully in case of errors, returning a user-friendly structure.
Implementation is coherent. Consider verifying that any paused environment’s concurrency remains zero if there are subsequent calls that could inadvertently restore concurrency.
apps/webapp/app/presenters/v3/QueueListPresenter.server.ts (1)
7-15: Consider makingperPageconfigurable.Currently,
DEFAULT_ITEMS_PER_PAGEis 25 and capped atMAX_ITEMS_PER_PAGEof 100. Depending on usage, you might want to allow these bounds to be set dynamically or based on user preferences.internal-packages/run-engine/src/run-queue/index.ts (1)
187-222: Handle partial errors in pipeline results more robustly.Right now, a pipeline error sets concurrency to 0, which might hide real concurrency usage. Consider distinguishing between actual zero usage and partial pipeline errors to ensure more accurate reporting.
apps/webapp/app/presenters/SelectBestEnvironmentPresenter.server.ts (1)
142-144: Consider skipping paused environments inselectBestEnvironment.The new method signature is comprehensive, but it doesn't filter out paused environments. If that is desired behavior, add a check here to pick only active environments.
apps/webapp/app/v3/services/pauseQueue.server.ts (3)
10-22: Consider adding more specific server-side error codes.The return object’s
codefield currently has only a few enumerated error strings ("queue-not-found","unknown-error","engine-version"). If there is any plan for more detailed server-side error handling (e.g. concurrency conflicts, database write failures), you might consider expanding this set for improved clarity and debugging.
60-68: Mitigate potential concurrency race conditions.While updating concurrency limits, if multiple “pause” or “resume” actions hit the server concurrently, you might risk race conditions. Consider using transactions or row-level locks for concurrency limit updates to ensure consistent results.
94-105: Enhance error handling and logging.Catching the unknown error is great. For additional observability, consider including more context (e.g., queue name, concurrency limit, environment details) in the logged error, so you can diagnose issues more effectively.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/webapp/app/assets/images/queues-dashboard.pngis excluded by!**/*.png
📒 Files selected for processing (54)
.gitignore(1 hunks).vscode/settings.json(1 hunks)apps/webapp/app/components/SetupCommands.tsx(0 hunks)apps/webapp/app/components/admin/debugTooltip.tsx(3 hunks)apps/webapp/app/components/layout/AppLayout.tsx(1 hunks)apps/webapp/app/components/metrics/BigNumber.tsx(1 hunks)apps/webapp/app/components/navigation/AccountSideMenu.tsx(1 hunks)apps/webapp/app/components/navigation/EnvironmentPausedBanner.tsx(1 hunks)apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx(2 hunks)apps/webapp/app/components/navigation/SideMenu.tsx(2 hunks)apps/webapp/app/components/primitives/AnimatedNumber.tsx(1 hunks)apps/webapp/app/components/primitives/Badge.tsx(1 hunks)apps/webapp/app/components/primitives/PageHeader.tsx(3 hunks)apps/webapp/app/components/runs/v3/RunIcon.tsx(2 hunks)apps/webapp/app/env.server.ts(1 hunks)apps/webapp/app/models/runtimeEnvironment.server.ts(2 hunks)apps/webapp/app/presenters/OrganizationsPresenter.server.ts(1 hunks)apps/webapp/app/presenters/SelectBestEnvironmentPresenter.server.ts(4 hunks)apps/webapp/app/presenters/v3/ConcurrencyPresenter.server.ts(0 hunks)apps/webapp/app/presenters/v3/EnvironmentQueuePresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/QueueListPresenter.server.ts(1 hunks)apps/webapp/app/presenters/v3/QueueRetrievePresenter.server.ts(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.concurrency/route.tsx(0 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsx(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug/route.tsx(2 hunks)apps/webapp/app/routes/api.v1.queues.$queueParam.pause.ts(1 hunks)apps/webapp/app/routes/api.v1.queues.$queueParam.ts(1 hunks)apps/webapp/app/routes/api.v1.queues.ts(1 hunks)apps/webapp/app/routes/orgs.$organizationSlug.projects.$projectParam.concurrency.ts(2 hunks)apps/webapp/app/routes/orgs.$organizationSlug.projects.$projectParam.env.$envParam.concurrency.ts(1 hunks)apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues.stream.tsx(1 hunks)apps/webapp/app/routes/storybook.info-panel/route.tsx(1 hunks)apps/webapp/app/routes/storybook/route.tsx(1 hunks)apps/webapp/app/services/routeBuilders/apiBuilder.server.ts(2 hunks)apps/webapp/app/utils/pathBuilder.ts(1 hunks)apps/webapp/app/utils/sse.ts(4 hunks)apps/webapp/app/v3/engineVersion.server.ts(2 hunks)apps/webapp/app/v3/runQueue.server.ts(1 hunks)apps/webapp/app/v3/services/pauseEnvironment.server.ts(1 hunks)apps/webapp/app/v3/services/pauseQueue.server.ts(1 hunks)internal-packages/database/prisma/migrations/20250318090847_pause_queues_and_environments/migration.sql(1 hunks)internal-packages/database/prisma/schema.prisma(2 hunks)internal-packages/run-engine/src/engine/index.ts(1 hunks)internal-packages/run-engine/src/run-queue/index.ts(1 hunks)packages/core/src/index.ts(1 hunks)packages/core/src/v3/apiClient/index.ts(2 hunks)packages/core/src/v3/schemas/index.ts(1 hunks)packages/core/src/v3/schemas/queues.ts(1 hunks)packages/react-hooks/src/package.json(0 hunks)packages/trigger-sdk/src/v3/index.ts(1 hunks)packages/trigger-sdk/src/v3/queues.ts(1 hunks)references/hello-world/src/trigger/queues.ts(1 hunks)
💤 Files with no reviewable changes (4)
- packages/react-hooks/src/package.json
- apps/webapp/app/components/SetupCommands.tsx
- apps/webapp/app/presenters/v3/ConcurrencyPresenter.server.ts
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.concurrency/route.tsx
🧰 Additional context used
🧬 Code Definitions (17)
apps/webapp/app/routes/orgs.$organizationSlug.projects.$projectParam.env.$envParam.concurrency.ts (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (1) (1)
loader(76-119)apps/webapp/app/routes/orgs.$organizationSlug.projects.$projectParam.concurrency.ts (1) (1)
loader(7-43)apps/webapp/app/utils/pathBuilder.ts (2) (2)
EnvironmentParamSchema(26-28)v3QueuesPath(306-312)
apps/webapp/app/routes/orgs.$organizationSlug.projects.$projectParam.concurrency.ts (1)
apps/webapp/app/utils/pathBuilder.ts (1) (1)
v3QueuesPath(306-312)
apps/webapp/app/routes/api.v1.queues.ts (3)
apps/webapp/app/services/routeBuilders/apiBuilder.server.ts (1) (1)
createLoaderApiRoute(92-279)apps/webapp/app/presenters/v3/QueueListPresenter.server.ts (1) (1)
QueueListPresenter(9-96)packages/core/src/v3/schemas/queues.ts (2) (2)
QueueItem(16-35)QueueItem(37-37)
apps/webapp/app/v3/services/pauseEnvironment.server.ts (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (1) (1)
action(121-197)apps/webapp/app/v3/runQueue.server.ts (1) (1)
updateEnvConcurrencyLimits(8-21)
apps/webapp/app/models/runtimeEnvironment.server.ts (1)
apps/webapp/app/presenters/OrganizationsPresenter.server.ts (1) (1)
userId(122-164)
apps/webapp/app/routes/api.v1.queues.$queueParam.ts (4)
packages/core/src/v3/schemas/queues.ts (6) (6)
RetrieveQueueType(13-13)RetrieveQueueType(14-14)RetrieveQueueParam(66-76)RetrieveQueueParam(78-78)QueueItem(16-35)QueueItem(37-37)apps/webapp/app/routes/api.v1.queues.ts (1) (1)
loader(13-44)apps/webapp/app/services/routeBuilders/apiBuilder.server.ts (1) (1)
createLoaderApiRoute(92-279)apps/webapp/app/presenters/v3/QueueRetrievePresenter.server.ts (1) (1)
QueueRetrievePresenter(37-82)
apps/webapp/app/components/metrics/BigNumber.tsx (1)
apps/webapp/app/components/primitives/AnimatedNumber.tsx (1) (1)
AnimatedNumber(4-13)
apps/webapp/app/routes/api.v1.queues.$queueParam.pause.ts (1)
apps/webapp/app/v3/services/pauseQueue.server.ts (1) (1)
PauseQueueService(24-107)
apps/webapp/app/v3/services/pauseQueue.server.ts (3)
packages/core/src/v3/schemas/queues.ts (4) (4)
QueueItem(16-35)QueueItem(37-37)RetrieveQueueParam(66-76)RetrieveQueueParam(78-78)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (1) (1)
action(121-197)apps/webapp/app/presenters/v3/QueueRetrievePresenter.server.ts (2) (2)
getQueue(13-35)toQueueItem(100-119)
apps/webapp/app/routes/_app.orgs.$organizationSlug/route.tsx (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (1) (1)
isEnvironmentPauseResumeFormSubmission(602-615)
apps/webapp/app/services/routeBuilders/apiBuilder.server.ts (1)
apps/webapp/app/services/apiAuth.server.ts (1) (1)
ApiAuthenticationResultSuccess(39-46)
packages/trigger-sdk/src/v3/queues.ts (2)
packages/core/src/v3/schemas/queues.ts (6) (6)
ListQueueOptions(39-44)ListQueueOptions(46-46)QueueItem(16-35)QueueItem(37-37)RetrieveQueueParam(66-76)RetrieveQueueParam(78-78)packages/core/src/v3/apiClient/index.ts (2) (2)
ApiRequestOptions(113-113)mergeRequestOptions(1017-1033)
apps/webapp/app/presenters/SelectBestEnvironmentPresenter.server.ts (2)
apps/webapp/app/models/runtimeEnvironment.server.ts (1) (1)
RuntimeEnvironment(6-6)apps/webapp/app/presenters/OrganizationsPresenter.server.ts (1) (1)
user(166-214)
apps/webapp/app/components/primitives/PageHeader.tsx (1)
apps/webapp/app/components/navigation/EnvironmentPausedBanner.tsx (1) (1)
EnvironmentPausedBanner(13-51)
apps/webapp/app/presenters/v3/QueueRetrievePresenter.server.ts (1)
packages/core/src/v3/schemas/queues.ts (4) (4)
RetrieveQueueParam(66-76)RetrieveQueueParam(78-78)QueueItem(16-35)QueueItem(37-37)
packages/core/src/v3/apiClient/index.ts (2)
packages/core/src/v3/schemas/queues.ts (6) (6)
ListQueueOptions(39-44)ListQueueOptions(46-46)QueueItem(16-35)QueueItem(37-37)RetrieveQueueParam(66-76)RetrieveQueueParam(78-78)packages/core/src/v3/apiClient/core.ts (3) (3)
ZodFetchOptions(30-38)zodfetchOffsetLimitPage(116-148)zodfetch(70-77)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (4)
apps/webapp/app/presenters/v3/QueueListPresenter.server.ts (1) (1)
QueueListPresenter(9-96)apps/webapp/app/presenters/v3/EnvironmentQueuePresenter.server.ts (1) (1)
EnvironmentQueuePresenter(12-30)apps/webapp/app/v3/services/pauseEnvironment.server.ts (1) (1)
PauseEnvironmentService(20-68)apps/webapp/app/v3/services/pauseQueue.server.ts (1) (1)
PauseQueueService(24-107)
🪛 Biome (1.9.4)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 163-163: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
🔇 Additional comments (85)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments/route.tsx (2)
51-51: Good addition of the titleCase utilityThe addition of the titleCase utility function from the shared utils package is a good choice for standardizing text formatting across the application.
178-178: UI enhancement with proper text formattingApplying titleCase to deployment labels improves readability and ensures consistent presentation of labels throughout the UI. This change enhances the user experience by standardizing the display format for deployment labels.
.vscode/settings.json (1)
5-10: Disable Vitest Workspace WarningThe new setting
"vitest.disableWorkspaceWarning": truewill help reduce unnecessary warnings and improve the developer experience when using Vitest. Ensure that this configuration is consistent with team policies and that no important warnings are inadvertently suppressed.apps/webapp/app/routes/orgs.$organizationSlug.projects.$projectParam.concurrency.ts (2)
5-5: Update to imports to support the new queues pathThe import has been updated to include
v3QueuesPathfrom the path builder utility, which is consistent with the PR objective of replacing the "Concurrency limits" page with a new "Queues" page.
42-42: Appropriate redirection from old concurrency route to new queues routeThis change implements a redirect from the old concurrency limits page to the new queues page, ensuring users are seamlessly guided to the updated UI. The implementation correctly uses the new
v3QueuesPathfunction with the required parameters (organization, project, and environment).This redirection strategy maintains backward compatibility while implementing the new feature described in the PR objectives.
apps/webapp/app/routes/storybook.info-panel/route.tsx (1)
1-118: Well-structured storybook component with comprehensive InfoPanel examplesThe implementation provides an excellent showcase of the InfoPanel component with various configurations, making it a valuable reference for developers. The examples demonstrate different variants, icons, colors, and button options in a clean, organized grid layout.
apps/webapp/app/routes/storybook/route.tsx (1)
55-58: Correctly integrated new Info panel story entryThe addition of the Info panel entry to the stories array follows the existing pattern and positioning convention, ensuring it will appear properly in the storybook navigation.
apps/webapp/app/components/runs/v3/RunIcon.tsx (1)
5-5: Clean implementation of queue icon.The addition of the
RectangleStackIconand its usage for queue representation with a purple color scheme is well-implemented and follows the existing pattern for icon rendering.Also applies to: 63-64
apps/webapp/app/utils/pathBuilder.ts (1)
306-312: LGTM - Proper path function implementation.This function follows the established pattern of other path builder functions in the file, correctly leveraging the environment path and appending the appropriate segment. This effectively replaces the previous concurrency limits functionality with the new queues page as mentioned in the PR objectives.
packages/core/src/index.ts (1)
4-4: Clean addition to expose version information.Adding this export makes version information available to consumers of the core package, which seems appropriate for the queue management features being added in this PR.
packages/trigger-sdk/src/v3/index.ts (1)
51-51: Well-organized export for the new queues functionality.The export follows the established pattern in the file, making the queues functionality accessible through the SDK. This aligns perfectly with the PR objectives of updating the SDK to support queue operations like listing, retrieving, pausing, and resuming.
apps/webapp/app/routes/orgs.$organizationSlug.projects.$projectParam.env.$envParam.concurrency.ts (1)
1-9: Effective redirect implementation for page transitionThis loader correctly implements a redirect from the old "Concurrency limits" route to the new "Queues" route, ensuring a seamless transition for users. The implementation properly extracts and validates URL parameters before constructing the redirect URL.
internal-packages/database/prisma/migrations/20250318090847_pause_queues_and_environments/migration.sql (1)
1-7: Migration adds necessary fields for pausing functionalityThe migration correctly adds a
pausedboolean column to both theRuntimeEnvironmentandTaskQueuetables, which aligns with the PR objectives of allowing users to pause/resume environments and queues. The default value offalseis appropriate as it ensures existing environments and queues will be in a running state by default.apps/webapp/app/components/admin/debugTooltip.tsx (3)
9-9: New environment hook added correctlyThe import for
useOptionalEnvironmentis properly added to support the new environment debug information.
39-39: Environment hook usage is appropriateThe hook is correctly used alongside the existing organization and project hooks.
67-82: Environment debug information well-structuredThe added environment information follows the same pattern as the existing organization and project information, maintaining consistency in the UI. The conditional rendering ensures the information only appears when an environment is available, and the paused state is properly displayed as "Yes" or "No".
apps/webapp/app/presenters/OrganizationsPresenter.server.ts (1)
78-78: Paused field correctly added to environment selectionThe
pausedfield is correctly added to the environment selection, which is necessary to expose this information to the frontend components that need to display and manage the paused state of environments.apps/webapp/app/components/navigation/AccountSideMenu.tsx (1)
26-26: Styling approach updated for consistencyThe "Back to app" text is now wrapped in a span with a specific text class, which aligns with styling patterns used in other similar components like the OrganizationSettingsSideMenu. This change improves styling consistency across the application.
apps/webapp/app/components/primitives/AnimatedNumber.tsx (1)
1-13: Well-implemented animation component using framer-motionThis component nicely encapsulates numeric value animation with smooth transitions. The spring animation parameters are well-tuned, and the component correctly updates animations when prop values change.
apps/webapp/app/routes/api.v1.queues.$queueParam.ts (3)
7-9: Good default value setup for the search paramsUsing the default value of "id" for the type parameter is a good approach, making the API more user-friendly by not requiring this parameter for common use cases.
11-18: API route setup follows best practicesThe loader setup with proper param validation and transformation is well-structured. The replacement of encoded slashes (%2F) is essential for handling path segments properly.
20-26: Clear input construction logicThe code effectively differentiates between ID-based and type-based queue retrieval, constructing the appropriate input object based on the
typeparameter.apps/webapp/app/components/navigation/SideMenu.tsx (2)
53-53: Added import for the new queues path builder functionThis correctly adds the required import to support the new Queues menu item.
192-198: Added Queues navigation menu itemThe new Queues menu item is correctly positioned in the navigation structure and follows the established pattern for menu items, including proper icon, color, and data-action attributes.
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings/route.tsx (4)
2-4: Added necessary imports for version displayThe new imports support retrieving and using the application version information.
9-13: Added loader to provide version informationThe loader effectively retrieves the VERSION constant and makes it available to the component via typedjson.
16-16: Updated component to use the version dataThe Page component now properly destructures and utilizes the version information from the loader.
22-22: Passing version prop to the settings sidebarThe version information is correctly passed to the OrganizationSettingsSideMenu component, enabling it to display version information.
apps/webapp/app/components/navigation/EnvironmentPausedBanner.tsx (2)
13-51: Clean implementation of the environment paused notification banner.The banner implementation is well-structured with good conditional rendering to only show when an environment is paused. The use of animation with framer-motion provides a smooth user experience when the banner appears or disappears.
53-57: Well-implemented custom hook for banner visibility logic.This hook correctly abstracts the visibility logic and makes it reusable across the application. The default value of
falseensures the UI won't show the banner unexpectedly if the environment data is not available.internal-packages/database/prisma/schema.prisma (2)
385-387: Good addition of pause capability to runtime environments.The
pausedfield with a default value offalseis appropriate for this feature and aligns with the PR's objective of allowing environment-level pausing.
2527-2528: Consistent implementation of pause capability for task queues.The same
pausedfield pattern is applied to theTaskQueuemodel, which maintains consistency with the environment-level implementation while enabling more granular control at the queue level.apps/webapp/app/utils/sse.ts (3)
56-74: Improved error handling for SSE events.The
createSafeSendfunction adds robust error handling to prevent issues when sending events after connections are closed. It properly:
- Checks if the controller is aborted before attempting to send
- Silently handles "Controller is already closed" errors
- Logs other errors for debugging
- Re-throws unexpected errors to maintain proper error propagation
This change will significantly improve the stability of the SSE implementation.
138-138: Good implementation of safe send wrapper.Creating the wrapper at the appropriate point in the event stream setup ensures all subsequent event sending is protected by the enhanced error handling.
144-144: Consistent application of safe send across all handlers.The implementation correctly replaces all instances of the original
sendfunction with the wrappedsafeSendfunction across all event handlers, ensuring consistent error handling throughout the SSE lifecycle.Also applies to: 162-162, 197-197
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues.stream.tsx (2)
8-11: Good configuration of the SSE loader with environment variables.The loader is properly configured with timeout and interval values from environment variables, allowing for easy configuration changes without code modifications.
12-32: Thorough validation and environment lookup logic.The handler correctly:
- Requires user authentication
- Validates route parameters using a schema
- Performs a database query with proper filtering to ensure the environment exists and belongs to the user
- Returns an appropriate 404 response if the environment is not found
This ensures that only authorized users can access the SSE stream for their environments.
apps/webapp/app/v3/runQueue.server.ts (2)
8-15: Function signature updated correctly to support environment pausingThe function now accepts an optional
maximumConcurrencyLimitparameter which allows setting the concurrency limit to pause or resume environments. The implementation correctly handles the case when the parameter is undefined by using a conditional check.
17-19: Good use of updatedEnvironment in the Promise.allSettled callsThe code properly uses the updated environment object in both async calls, ensuring consistency between MARQS and the RunQueue engine.
apps/webapp/app/routes/api.v1.queues.ts (3)
8-11: Well-defined input validation with ZodThe SearchParamsSchema correctly validates pagination parameters, using coercion for numeric types and making both optional with appropriate constraints.
18-29: Good error handling in the loader functionThe code correctly initializes the QueueListPresenter with the optional perPage parameter and properly handles the case when the result is unsuccessful.
31-42: Comprehensive error handling with appropriate status codesThe implementation correctly catches different types of errors and returns appropriate status codes. The use of
ServiceValidationErrorwith a 422 status is particularly good for handling validation errors distinctly from other errors.apps/webapp/app/routes/_app.orgs.$organizationSlug/route.tsx (2)
14-15: Correctly added new imports for enhanced functionalityThe imports for
isEnvironmentPauseResumeFormSubmissionandloggerare correctly added. Theloggerimport is necessary for consistent logging throughout the application.
50-53: Good implementation of route revalidation for environment pause/resumeThis addition correctly implements route revalidation when an environment is paused or resumed, ensuring the UI updates appropriately. The comment explains the purpose of the code, and the implementation aligns with the existing pattern of the
shouldRevalidatefunction.apps/webapp/app/components/primitives/PageHeader.tsx (2)
2-2: Type import optimizationGood use of the
typekeyword to only import the type definition for ReactNode, which can help with tree-shaking.
8-8: Component import for environment status indicationThe import of
EnvironmentPausedBanneris appropriately added to support the new feature for showing when an environment is paused.apps/webapp/app/routes/api.v1.queues.$queueParam.pause.ts (1)
1-46: Well-structured API route for pausing/resuming queues.This implementation is well-designed with proper validation, error handling, and separation of concerns. The route correctly validates inputs, transforms encoded parameters, and handles different error scenarios appropriately.
apps/webapp/app/models/runtimeEnvironment.server.ts (1)
72-99: Good improvement to the function signature and data retrieval.The updated function signature with explicit return type enhances type safety, while the added include clause optimizes data retrieval by fetching related entities in a single query rather than requiring multiple queries.
apps/webapp/app/v3/engineVersion.server.ts (2)
8-15: Good addition of explicit Environment type.Creating this explicit type enhances type safety and code readability by clearly defining the expected structure of environment objects used in the determineEngineVersion function.
22-23: Consistent parameter type update.The parameter type change from
AuthenticatedEnvironmenttoEnvironmentand the corresponding update fromenvironment.projectIdtoenvironment.project.idensure consistency with the newly defined type structure.Also applies to: 47-48
apps/webapp/app/components/metrics/BigNumber.tsx (5)
6-16: Well-structured props interfaceThe
BigNumberPropsinterface is clearly defined with appropriate types and optional flags, making the component API easy to understand and use.
18-28: Clean component implementation with sensible defaultsThe component function is well-implemented with destructured props and default values for
animateandloading. The parameter list matches the interface definition.
31-35: Good layout structure with semantic class namesThe layout uses a grid structure with descriptive Tailwind class names that clearly indicate the component's visual hierarchy.
36-41: Properly using className utility for conditional stylingThe component correctly uses the
cnutility for class name composition, allowing for custom styling through thevalueClassNameprop.
42-51: Well-handled conditional rendering based on component stateThe component elegantly handles different states with a ternary operator cascade:
- Shows a spinner when loading
- Renders the value (animated or static) when available
- Falls back to a placeholder dash when value is undefined
Good use of the
AnimatedNumbercomponent from the related file.apps/webapp/app/services/routeBuilders/apiBuilder.server.ts (2)
42-47: Properly extended API route functionality with search params supportThe
findResourcefunction signature has been enhanced to accept search parameters, which aligns with the PR objective of adding queue management functionality. The type definition is correctly using conditional typing based on the schema type.
186-187: Updated function call matches the extended function signatureThe call to
findResourcehas been properly updated to pass theparsedSearchParamsas the third argument, consistent with the updated function signature.apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx (5)
23-23: Added Paragraph component import for version displayThe new import is needed for displaying the app version information in the side menu.
25-31: Added version prop to component interfaceThe component now accepts a
versionprop of type string, which is used to display the application version in the UI.
49-49: Improved accessibility by wrapping text in a spanThe "Back to app" text is now properly wrapped in a span element with appropriate styling, which is a good practice for accessibility and styling flexibility.
52-52: Enhanced spacing for better visual hierarchyThe gap in the flex layout has been increased from 1 to 4 units, providing better visual separation between groups of related items.
91-96: Well-implemented version display sectionThe version information is nicely presented in its own section with a proper header and formatting. The version number is displayed with a "v" prefix for clarity.
internal-packages/run-engine/src/engine/index.ts (2)
1659-1665: Clean delegation torunQueue.lengthOfQueues.This code simply delegates to the underlying
runQueuemethod. Looks good for modularity and readability.
1666-1672: Confirm correct queue concurrency retrieval.Like the previous function, this method delegates to
runQueue.currentConcurrencyOfQueues. It might be beneficial to ensure that the result properly accounts for any paused or partially paused queues if such a state exists.Would you like a script to check whether paused queues are filtered out or included in concurrency calculations?
apps/webapp/app/presenters/v3/EnvironmentQueuePresenter.server.ts (2)
1-5: Validate import usage and dependency structure.All imports appear valid; however, ensure these dependencies (
marqs,engine) are always available for the environment in which this presenter runs.
12-30: Useful combined concurrency logic with fallback checks.This class concisely merges concurrency data from both engines. If the environment can be paused, it might be valuable to guard logic or show that paused states yield zero concurrency or zero queues. That aside, the implementation is clear and succinct.
apps/webapp/app/v3/services/pauseEnvironment.server.ts (3)
1-7: Imports and base class integration look appropriate.The injection of
PrismaClientOrTransactionand usage of the base service design pattern is well-structured. No issues found.
8-19: Clear definitions for pause-related types.The
PauseStatusandPauseEnvironmentResulttypes are simple and effective for conveying pause/resume environment states.
20-24: Constructor neatly initializes the Prisma instance.The constructor ensures the Prisma instance is properly passed up to the base class. This is clean and consistent.
apps/webapp/app/presenters/v3/QueueListPresenter.server.ts (1)
53-95: Verify concurrency handling before fetching queue data.Before calling
engine.lengthOfQueues()orengine.currentConcurrencyOfQueues(), ensure environment concurrency configurations are valid to avoid unexpected runtime or Redis pipeline errors.apps/webapp/app/presenters/SelectBestEnvironmentPresenter.server.ts (1)
10-10: Leverage the newpausedproperty carefully.You have added the
pausedfield toMinimumEnvironment. If environments are paused, you may want to conditionally exclude them from showing up or add a notice to users. Verify whether the existing logic properly handles the paused state.apps/webapp/app/v3/services/pauseQueue.server.ts (1)
30-41: Double-check environment engine compatibility.The approach to check
engineVersionhere is correct. However, consider verifying that older engine versions (like “V1”) cannot accidentally proceed to the concurrency update lines if an asynchronous state or edge case emerges. This can help prevent partial updates when the environment is in an invalid version state.packages/core/src/v3/apiClient/index.ts (4)
21-21: Newly imported types look consistent and appropriate.These additions for
ListQueueOptions,QueueItem, andRetrieveQueueParamcorrectly align with the queue functionality introduced in this PR.Also applies to: 24-24, 28-28
722-746:listQueuesmethod appears well-structured.• Correctly maps
pageandperPageinto query parameters and leverageszodfetchOffsetLimitPagefor paginated data.
• The usage ofQueueItemschema ensures consistent validation of the retrieved queues.
748-764: Logical approach for retrieving a single queue.• Encoding slashes prevents URL misinterpretations for nested queue names.
• Conditional handling for string vs. object is clear and aligns withRetrieveQueueParam.
• Consider logging or confirming a 404 downstream, if retrieval fails, to aid debugging.
766-790:pauseQueuemethod cleanly manages pause/resume actions.• Consistently encodes queue identifiers and sets the action in the request body.
• Proper separation of "pause" vs. "resume" states.
• Suggest verifying or handling potential concurrency collisions (e.g., two simultaneous "pause" calls) elsewhere in the system.apps/webapp/app/presenters/v3/QueueRetrievePresenter.server.ts (5)
1-9: Imports look appropriate and concise.All imported modules are relevant to queue retrieval and environment checks.
10-35:getQueuefunction effectively handles string vs. object param forms.• Uses the
friendlyIdcheck for the string case and a normalizednamefor the object case.
• Returnsnullif not found, leaving higher-level logic to handle missing queues.
37-82:QueueRetrievePresenterprovides clear logic for retrieving and presenting a queue.• Properly checks for engine version compatibility and returns an error if V1.
• Aggregates queue data (running, queued counts) viaengine.*OfQueuescalls.
• Final data is passed throughtoQueueItem, ensuring consistent shape.
84-93:queueTypeFromTypefunction ensures correct mapping to “custom” or “task.”• Proper usage of
assertExhaustiveto handle future expansions.
• Straightforward, no further issues.
95-119:toQueueItemreturns a properly validated queue object.• Strips "
task/" prefix and normalizes name/type.
• Returns the expectedQueueItemshape with concurrency and paused states.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (1)
602-616: Environment form-check helper is concise and correct.
isEnvironmentPauseResumeFormSubmissionproperly identifies pause/resume actions by verifying both method and relevant form data fields.packages/core/src/v3/schemas/index.ts (1)
17-17: New queues functionality correctly exportedThis export adds the queue management functions to the module interface, which aligns with the PR objectives of allowing users to view, pause, and resume queues. The placement and naming convention are consistent with the existing export statements.
apps/webapp/app/components/primitives/Badge.tsx (1)
10-10: UI refinement: Letter spacing removed from extra-small badgesRemoving the
tracking-wideclass will give extra-small badges normal letter spacing instead of wider spacing. This is a reasonable UI refinement that likely improves visual consistency.apps/webapp/app/components/layout/AppLayout.tsx (1)
14-14: Simplified layout with static grid structureThe
PageContainerhas been simplified to use a static grid layout instead of dynamically adjusting based on organization or upgrade prompts. This makes the layout more predictable and easier to maintain.The
grid-rows-[auto_1fr]pattern is appropriate here: the first row adapts to its content size, while the second row takes up the remaining space in the container.
| public async call({ | ||
| environment, | ||
| page, | ||
| }: { | ||
| environment: AuthenticatedEnvironment; | ||
| page: number; | ||
| perPage?: number; | ||
| }) { | ||
| //check the engine is the correct version | ||
| const engineVersion = await determineEngineVersion({ environment }); | ||
|
|
||
| if (engineVersion === "V1") { | ||
| return { | ||
| success: false as const, | ||
| code: "engine-version", | ||
| }; | ||
| } | ||
|
|
||
| // Get total count for pagination | ||
| const totalQueues = await this._replica.taskQueue.count({ | ||
| where: { | ||
| runtimeEnvironmentId: environment.id, | ||
| }, | ||
| }); | ||
|
|
||
| return { | ||
| success: true as const, | ||
| queues: await this.getQueuesWithPagination(environment, page), | ||
| pagination: { | ||
| currentPage: page, | ||
| totalPages: Math.ceil(totalQueues / this.perPage), | ||
| count: totalQueues, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate the page parameter to prevent out-of-range pagination.
Consider adding a guard clause to ensure page is at least 1. Otherwise, negative page values or zero could lead to confusing user experiences or runtime errors.
| switch (action) { | ||
| case "environment-pause": | ||
| const pauseService = new PauseEnvironmentService(); | ||
| await pauseService.call(environment, "paused"); | ||
| return redirectWithSuccessMessage(redirectPath, request, "Environment paused"); | ||
| case "environment-resume": | ||
| const resumeService = new PauseEnvironmentService(); | ||
| await resumeService.call(environment, "resumed"); | ||
| return redirectWithSuccessMessage(redirectPath, request, "Environment resumed"); | ||
| case "queue-pause": | ||
| case "queue-resume": { | ||
| const friendlyId = formData.get("friendlyId"); | ||
| if (!friendlyId) { | ||
| return redirectWithErrorMessage(redirectPath, request, "Queue ID is required"); | ||
| } | ||
|
|
||
| const queueService = new PauseQueueService(); | ||
| const result = await queueService.call( | ||
| environment, | ||
| friendlyId.toString(), | ||
| action === "queue-pause" ? "paused" : "resumed" | ||
| ); | ||
|
|
||
| if (!result.success) { | ||
| return redirectWithErrorMessage( | ||
| redirectPath, | ||
| request, | ||
| result.error ?? `Failed to ${action === "queue-pause" ? "pause" : "resume"} queue` | ||
| ); | ||
| } | ||
|
|
||
| return redirectWithSuccessMessage( | ||
| redirectPath, | ||
| request, | ||
| `Queue ${action === "queue-pause" ? "paused" : "resumed"}` | ||
| ); | ||
| } | ||
| default: | ||
| return redirectWithErrorMessage(redirectPath, request, "Something went wrong"); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Encapsulate variable declarations within each switch case block.
Static analysis indicates that variables like pauseService and resumeService can potentially “leak” into other branches. To avoid any unexpected reference across cases, wrap these declarations in braces:
switch (action) {
case "environment-pause": {
- const pauseService = new PauseEnvironmentService();
+ const pauseService = new PauseEnvironmentService();
await pauseService.call(environment, "paused");
return redirectWithSuccessMessage(redirectPath, request, "Environment paused");
}
case "environment-resume": {
- const resumeService = new PauseEnvironmentService();
+ const resumeService = new PauseEnvironmentService();
await resumeService.call(environment, "resumed");
return redirectWithSuccessMessage(redirectPath, request, "Environment resumed");
}
...
}This ensures proper scoping and resolves the warning.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| switch (action) { | |
| case "environment-pause": | |
| const pauseService = new PauseEnvironmentService(); | |
| await pauseService.call(environment, "paused"); | |
| return redirectWithSuccessMessage(redirectPath, request, "Environment paused"); | |
| case "environment-resume": | |
| const resumeService = new PauseEnvironmentService(); | |
| await resumeService.call(environment, "resumed"); | |
| return redirectWithSuccessMessage(redirectPath, request, "Environment resumed"); | |
| case "queue-pause": | |
| case "queue-resume": { | |
| const friendlyId = formData.get("friendlyId"); | |
| if (!friendlyId) { | |
| return redirectWithErrorMessage(redirectPath, request, "Queue ID is required"); | |
| } | |
| const queueService = new PauseQueueService(); | |
| const result = await queueService.call( | |
| environment, | |
| friendlyId.toString(), | |
| action === "queue-pause" ? "paused" : "resumed" | |
| ); | |
| if (!result.success) { | |
| return redirectWithErrorMessage( | |
| redirectPath, | |
| request, | |
| result.error ?? `Failed to ${action === "queue-pause" ? "pause" : "resume"} queue` | |
| ); | |
| } | |
| return redirectWithSuccessMessage( | |
| redirectPath, | |
| request, | |
| `Queue ${action === "queue-pause" ? "paused" : "resumed"}` | |
| ); | |
| } | |
| default: | |
| return redirectWithErrorMessage(redirectPath, request, "Something went wrong"); | |
| } | |
| }; | |
| switch (action) { | |
| case "environment-pause": { | |
| const pauseService = new PauseEnvironmentService(); | |
| await pauseService.call(environment, "paused"); | |
| return redirectWithSuccessMessage(redirectPath, request, "Environment paused"); | |
| } | |
| case "environment-resume": { | |
| const resumeService = new PauseEnvironmentService(); | |
| await resumeService.call(environment, "resumed"); | |
| return redirectWithSuccessMessage(redirectPath, request, "Environment resumed"); | |
| } | |
| case "queue-pause": | |
| case "queue-resume": { | |
| const friendlyId = formData.get("friendlyId"); | |
| if (!friendlyId) { | |
| return redirectWithErrorMessage(redirectPath, request, "Queue ID is required"); | |
| } | |
| const queueService = new PauseQueueService(); | |
| const result = await queueService.call( | |
| environment, | |
| friendlyId.toString(), | |
| action === "queue-pause" ? "paused" : "resumed" | |
| ); | |
| if (!result.success) { | |
| return redirectWithErrorMessage( | |
| redirectPath, | |
| request, | |
| result.error ?? `Failed to ${action === "queue-pause" ? "pause" : "resume"} queue` | |
| ); | |
| } | |
| return redirectWithSuccessMessage( | |
| redirectPath, | |
| request, | |
| `Queue ${action === "queue-pause" ? "paused" : "resumed"}` | |
| ); | |
| } | |
| default: | |
| return redirectWithErrorMessage(redirectPath, request, "Something went wrong"); | |
| } | |
| }; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 163-163: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (1)
509-584: Add navigation state handling for queue pause/resume button.The
QueuePauseResumeButtoncomponent lacks navigation state handling to close the dialog after form submission, unlike theEnvironmentPauseResumeButtoncomponent which includes this logic. This inconsistency could lead to the dialog staying open after a successful action.Add navigation state handling to ensure consistent behavior:
function QueuePauseResumeButton({ queue, }: { /** The "id" here is a friendlyId */ queue: { id: string; name: string; paused: boolean }; }) { const navigation = useNavigation(); const [isOpen, setIsOpen] = useState(false); + useEffect(() => { + if (navigation.state === "loading" || navigation.state === "idle") { + setIsOpen(false); + } + }, [navigation.state]); + + const isLoading = Boolean( + navigation.formData?.get("action") === (queue.paused ? "queue-resume" : "queue-pause") && + navigation.formData?.get("friendlyId") === queue.id + ); return ( <Dialog open={isOpen} onOpenChange={setIsOpen}> {/* ... rest of component ... */} <Button type="submit" + disabled={isLoading} shortcut={{ modifiers: ["mod"], key: "enter" }} variant={queue.paused ? "primary/medium" : "danger/medium"} - LeadingIcon={queue.paused ? PlayIcon : PauseIcon} + LeadingIcon={isLoading ? <Spinner /> : queue.paused ? PlayIcon : PauseIcon} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (5)
apps/webapp/app/models/runtimeEnvironment.server.ts (1) (1)
findEnvironmentBySlug(72-101)apps/webapp/app/presenters/v3/QueueListPresenter.server.ts (1) (1)
QueueListPresenter(9-96)apps/webapp/app/presenters/v3/EnvironmentQueuePresenter.server.ts (1) (1)
EnvironmentQueuePresenter(12-31)apps/webapp/app/v3/services/pauseEnvironment.server.ts (1) (1)
PauseEnvironmentService(20-68)apps/webapp/app/v3/services/pauseQueue.server.ts (1) (1)
PauseQueueService(24-107)
🪛 Biome (1.9.4)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 163-163: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
🔇 Additional comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (2)
157-166: Encapsulate variable declarations within each switch case block.Static analysis indicates that variables like
pauseServiceandresumeServicecan potentially "leak" into other branches. To avoid any unexpected reference across cases, wrap these declarations in braces:switch (action) { - case "environment-pause": - const pauseService = new PauseEnvironmentService(); + case "environment-pause": { + const pauseService = new PauseEnvironmentService(); await pauseService.call(environment, "paused"); return redirectWithSuccessMessage(redirectPath, request, "Environment paused"); + } - case "environment-resume": - const resumeService = new PauseEnvironmentService(); + case "environment-resume": { + const resumeService = new PauseEnvironmentService(); await resumeService.call(environment, "resumed"); return redirectWithSuccessMessage(redirectPath, request, "Environment resumed"); + }This ensures proper scoping and resolves the warning.
🧰 Tools
🪛 Biome (1.9.4)
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 163-163: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
1-624: Overall implementation LGTM!The implementation of the new Queues page looks robust with good error handling, clear UI structure, and appropriate feedback mechanisms. The code effectively handles pausing/resuming of both environments and individual queues, with proper permission checks and validation.
🧰 Tools
🪛 Biome (1.9.4)
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 163-163: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (3)
114-114: Use a structured logger for better traceability
Employingconsole.errormay suffice for small prototypes, but a centralized logger (e.g.,logger.error) with contextual data is often preferable for scalable error tracking and consistent formatting.- console.error(error); + logger.error("Loader error in route.tsx", { error });
169-172: Validate the queue ID more strictly
Relying onfriendlyIdfromformDatais functional but unguarded. Instead, consider applying a Zod schema or another validation mechanism for enhanced security and clearer error reporting when the ID is invalid.
618-631: Consider renaming or broadening this helper function
isEnvironmentPauseResumeFormSubmissionis narrowly targeted for environment pause/resume actions. If expansion or consolidation with queue actions is anticipated, either rename or enhance the function to cover both cases reliably.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/components/BlankStatePanels.tsx(2 hunks)apps/webapp/app/presenters/v3/QueueListPresenter.server.ts(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/presenters/v3/QueueListPresenter.server.ts
🧰 Additional context used
🧬 Code Definitions (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (3)
apps/webapp/app/models/runtimeEnvironment.server.ts (1) (1)
findEnvironmentBySlug(72-101)apps/webapp/app/presenters/v3/QueueListPresenter.server.ts (1) (1)
QueueListPresenter(9-98)apps/webapp/app/v3/services/pauseQueue.server.ts (1) (1)
PauseQueueService(24-107)
🪛 Biome (1.9.4)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
[error] 160-160: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 164-164: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/components/BlankStatePanels.tsx (2)
9-9: Good addition of required icon import.The newly imported
RectangleStackIconis appropriately added to support the new Queues functionality being introduced.
372-395: Well-structured blank state component for Queues.This new component follows the established pattern of other blank state components in the file. It properly retrieves the organizational context, presents a clear message about the empty state, and provides a relevant call-to-action that directs users to add tasks.
The component aligns well with the PR objectives of introducing a new Queues page to replace the Concurrency limits page.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx (2)
225-239: Neat modular layout with NavBar and PageBody
The use of<NavBar>and<PageBody>here, along with clearly defined sections, creates a straightforward and organized UI structure. This improves readability and maintainability of the route’s layout.
158-198: Enclose variable declarations in their case blocks
Switch-case variable declarations likepauseServiceandresumeServicecan unintentionally leak into other cases. Wrapping them in curly braces ensures block-level scoping.switch (action) { case "environment-pause": { + { const pauseService = new PauseEnvironmentService(); await pauseService.call(environment, "paused"); return redirectWithSuccessMessage(redirectPath, request, "Environment paused"); + } } case "environment-resume": { + { const resumeService = new PauseEnvironmentService(); await resumeService.call(environment, "resumed"); return redirectWithSuccessMessage(redirectPath, request, "Environment resumed"); + } } ... }🧰 Tools
🪛 Biome (1.9.4)
[error] 160-160: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 164-164: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
Queues page
We've added a new "Queues" page. This replaces the old "Concurrency limits" page.
Features
queuesSDK functionsAdded functions to list/retrieve/pause/resume individual queues
v4+ only
The Queues table and SDK functions will only work if you're on engine 2+ (SDK v4+).
Summary by CodeRabbit