Skip to content

Conversation

@jcgruenhage
Copy link
Member

I went into the codebase to add a subcommand for listing cells. As you can tell by the git diff --stat, I've gotten somewhat sidetracked with a refactor to get rid of some tech debt.

I've split this into smaller commits to make it easier to review.

The action.yml prevented running the `outliers` mode, even though the
CLI supported it already. Because it generally works I'd call this a bug
in the action metadata.
Our version schemes for dev and prod are disjunct, so it is implicitly
passed through that already.
@jcgruenhage jcgruenhage requested a review from a team as a code owner November 12, 2025 17:24
@jcgruenhage jcgruenhage requested review from fedordikarev and removed request for a team November 12, 2025 17:24
Copy link
Contributor

@fedordikarev fedordikarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jcgruenhage I am fine to approve without checking, if all of that is refactor.
Please let me know if there are part you want to have focused review or opinion.
If not, I will approve PR in 30 minutes (7pm CET)

@fedordikarev
Copy link
Contributor

@copilot please review the PR

Copy link

Copilot AI commented Nov 12, 2025

@fedordikarev I've opened a new pull request, #68, to work on those changes. Once the pull request is ready, I'll request review from you.

@jcgruenhage
Copy link
Member Author

@fedordikarev it's not all refactoring. If you look at the individual commits in here, you should be able to tell which ones are interesting to look at, feel free to skip the five marked as refactor or chore, those refactor, bump dependencies, reformat, etc. The remaining 5 are an order of magnitude less changes and should be easy to skim through.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for listing cells in the deploy-queue system while performing a significant refactor to reduce technical debt. The refactor reorganizes the codebase into a more modular structure with separate handler modules for different operations.

Key changes:

  • Introduces a new list cells subcommand to display known cells for a given environment
  • Refactors code organization by splitting monolithic lib.rs into focused modules (handler, model, util, constants)
  • Adds a materialized view for cells with automatic refresh on deployment insert

Reviewed Changes

Copilot reviewed 17 out of 27 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
deploy-queue/src/lib.rs Refactored main entry point to use new modular structure with separate handler modules
deploy-queue/src/model.rs Extracted data models (Cell, Deployment, DeploymentState, etc.) into dedicated module
deploy-queue/src/handler/mod.rs Created handler module with core deployment operations
deploy-queue/src/handler/fetch.rs Implements fetch operations for deployments, outliers, and cells
deploy-queue/src/handler/list.rs Implements listing functionality for outliers and cells
deploy-queue/src/handler/cancel.rs Implements cancellation operations for deployments
deploy-queue/src/util/mod.rs Created utility module for database, duration, and GitHub integrations
deploy-queue/src/util/duration.rs Extracted duration conversion utilities with DurationExt trait
deploy-queue/src/util/database.rs Extracted database connection logic
deploy-queue/src/util/github.rs Extracted GitHub Actions output utilities
deploy-queue/src/constants.rs Centralized timeout and retry constants
deploy-queue/src/cli.rs Updated CLI structure to support new list subcommand and refactored cancel options
deploy-queue/migrations/0007_cells_materialized_view.sql Added materialized view for distinct cells with auto-refresh trigger
deploy-queue/tests/integration_tests.rs Updated tests to use new handler API structure
deploy-queue/tests/outlier_detection_tests.rs Updated tests to use new handler API structure
deploy-queue/tests/deployment_analytics_tests.rs Updated tests to use new handler API structure
deploy-queue/action.yml Updated GitHub Action to support new CLI structure
deploy-queue/Cargo.toml Bumped version to 0.6.0
Files not reviewed (7)
  • deploy-queue/.sqlx/query-1096e32f72fa665a9d7617ebff0493a33882ffc11f77739434bb94b0503f37f6.json: Language not supported
  • deploy-queue/.sqlx/query-1283715454efddfcc6620af6d3fd49ee5704a30450a54f1cd313c7357d69f438.json: Language not supported
  • deploy-queue/.sqlx/query-305b0466fdb34e01fa1c7029b83fe410002c3b54331c7af27e2e82011b011172.json: Language not supported
  • deploy-queue/.sqlx/query-41bca488d310eb8e50fc7150a94342972d3561c060c246bf246812fb4b556b3e.json: Language not supported
  • deploy-queue/.sqlx/query-8e59cf17dcf58d773c0de3b1c023944c34484829592744e8bad2fd5fbbd621cd.json: Language not supported
  • deploy-queue/.sqlx/query-ee8e3e3db7349201dd037a83afac5c7875d33162e4c6320d18e66a2ff681dc33.json: Language not supported
  • deploy-queue/.sqlx/query-fd967284576d89af5aec4ac7c2a14917b4a21d6d205f56a5f98ac93dca1b3b5c.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jcgruenhage jcgruenhage force-pushed the jc.gruenhage/push-rkyvpnlqnyus branch from 2613491 to 80c7611 Compare November 12, 2025 17:38
@jcgruenhage jcgruenhage force-pushed the jc.gruenhage/push-rkyvpnlqnyus branch from 80c7611 to 781f934 Compare November 12, 2025 17:53
Copy link
Contributor

@fedordikarev fedordikarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jcgruenhage jcgruenhage merged commit 781f934 into main Nov 12, 2025
9 checks passed
@jcgruenhage jcgruenhage deleted the jc.gruenhage/push-rkyvpnlqnyus branch November 12, 2025 18:07
@jcgruenhage jcgruenhage mentioned this pull request Nov 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants