projects refresh background service #2011
Conversation
WalkthroughA new background service for refreshing project data from repositories was introduced, including its Dockerfile, deployment configuration, and implementation. The process of cloning repositories and refreshing projects has been refactored to use a shared Changes
Sequence Diagram(s)sequenceDiagram
participant User/API
participant Backend
participant ProjectsRefreshService
participant GitRepo
participant Database
User/API->>Backend: Request to refresh projects
Backend->>ProjectsRefreshService: TriggerProjectsRefreshService API call (with repo info)
ProjectsRefreshService->>GitRepo: Clone repository using git_utils
ProjectsRefreshService->>ProjectsRefreshService: Load digger.yml config
ProjectsRefreshService->>Database: Refresh projects in DB
ProjectsRefreshService-->>Backend: Respond with result
Backend-->>User/API: Return refresh status
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
✨ Finishing Touches
🧪 Generate unit tests
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 (
|
|
✅ No security or compliance issues detected. Reviewed everything up to 3a911ec. Security Overview
Detected Code Changes
Reply to this PR with |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces a new background service for refreshing projects configuration, moving Git operations to a centralized package, and implementing asynchronous project updates via Fly.io machines.
- Introduced
background/projects-refresh-service/projects_refesh_main.godaemon process for handling repository cloning and config updates asynchronously - Moved Git operations to centralized
libs/git_utils/clone_utils.gowith enhanced logging and error handling - Added
backend/service_clients/projects_service.gofor triggering on-demand Fly.io machines for project refresh operations - Added debug logging in
backend/models/storage.gofor improved project refresh operation visibility - Refactored
backend/services/repos.goto delegate project refresh logic to the new background service
22 files reviewed, 21 comments
Edit PR Review Bot Settings | Greptile
| @@ -0,0 +1,3 @@ | |||
| module github.com/diggerhq/digger/backgorund/projects-refresh-service | |||
There was a problem hiding this comment.
logic: Module path contains typo: 'backgorund' should be 'background'
| module github.com/diggerhq/digger/backgorund/projects-refresh-service | |
| module github.com/diggerhq/digger/background/projects-refresh-service |
| @@ -0,0 +1,3 @@ | |||
| module github.com/diggerhq/digger/backgorund/projects-refresh-service | |||
|
|
|||
| go 1.24.0 | |||
There was a problem hiding this comment.
logic: Using Go 1.24.0 which is not a valid Go version. Latest stable release is 1.22.x
| go 1.24.0 | |
| go 1.22.0 |
| ARG COMMIT_SHA | ||
| WORKDIR /app | ||
|
|
||
| RUN apt-get update && apt-get install -y ca-certificates curl && apt-get install -y git && apt-get clean all |
There was a problem hiding this comment.
style: Add --no-install-recommends to apt-get install to reduce image size (consistent with other Dockerfiles)
| RUN apt-get update && apt-get install -y ca-certificates curl && apt-get install -y git && apt-get clean all | |
| RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates curl git && apt-get clean all |
| # Copy all required source, blacklist files that are not required through `.dockerignore` | ||
| COPY . . | ||
|
|
||
| RUN go build -ldflags="-X 'main.Version=${COMMIT_SHA}'" -o projects_refresh_exe ./background/projects-refresh-service |
There was a problem hiding this comment.
style: Add 'go mod download' before build step to ensure dependencies are properly cached
| [http_service] | ||
| internal_port = 8080 | ||
| force_https = true | ||
| auto_stop_machines = 'stop' |
There was a problem hiding this comment.
syntax: Value 'stop' is invalid for auto_stop_machines. Should be boolean true/false.
| auto_stop_machines = 'stop' | |
| auto_stop_machines = true |
| if cloneUrl == "" && branch == "" && token == "" && orgId == "" && repoFullName == "" { | ||
| slog.Info("smoketests mode, skipping this run") | ||
| os.Exit(0) | ||
| } |
There was a problem hiding this comment.
logic: Exit code 0 when required env vars are missing may mask deployment issues. Consider requiring at least orgId and repoFullName. Validate vars individually for better error messages.
| if cloneUrl == "" && branch == "" && token == "" && orgId == "" && repoFullName == "" { | |
| slog.Info("smoketests mode, skipping this run") | |
| os.Exit(0) | |
| } | |
| if orgId == "" || repoFullName == "" { | |
| slog.Error("required environment variables missing", "orgId", orgId == "", "repoFullName", repoFullName == "") | |
| os.Exit(1) | |
| } | |
| if cloneUrl == "" && branch == "" && token == "" { | |
| slog.Info("smoketests mode, skipping this run") | |
| os.Exit(0) | |
| } |
| slog.Error("failed to load digger.yml: %v", "error", err) | ||
| return fmt.Errorf("error loading digger.yml %v", err) |
There was a problem hiding this comment.
syntax: Error string formatting uses %v twice incorrectly. The first should be plain text.
| slog.Error("failed to load digger.yml: %v", "error", err) | |
| return fmt.Errorf("error loading digger.yml %v", err) | |
| slog.Error("failed to load digger.yml", "error", err) | |
| return fmt.Errorf("error loading digger.yml: %v", err) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
background/projects-refresh-service/.gitignore (1)
1-1:.gitignorepattern is overly broadUsing a bare
mainpattern will ignore any file or directory namedmainanywhere in the tree, which might unintentionally hide unrelated sources (e.g.pkg/main/helpers.go). To only ignore the compiled binary produced at the service root, anchor the pattern:-main +/mainnext/services/config.go (1)
6-6: Use meaningful alias instead ofutils3The alias
utils3does not communicate intent and suggests copy-paste residue. Import the package unaliased or asgitutilsfor clarity:- utils3 "github.com/diggerhq/digger/libs/git_utils" + gitutils "github.com/diggerhq/digger/libs/git_utils"Remember to update the call sites accordingly.
Dockerfile_bg_projects_refresh (2)
6-6: Consider using modern Go workspace instead of GOPATH.The
$GOPATHapproach is less common in modern Go. Consider using/appor similar as the working directory.-WORKDIR $GOPATH/src/github.com/diggerhq/digger +WORKDIR /app
18-18: Improve apt cleanup for smaller image size.The current cleanup could be more thorough to reduce image size.
-RUN apt-get update && apt-get install -y ca-certificates curl && apt-get install -y git && apt-get clean all +RUN apt-get update && apt-get install -y ca-certificates curl git && \ + apt-get clean && rm -rf /var/lib/apt/lists/*background/projects-refresh-service/projects_refesh_main.go (1)
1-1: Fix the filename typo.The filename contains a typo: "refesh" should be "refresh". Consider renaming the file to
projects_refresh_main.gofor consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.workis excluded by!**/*.work
📒 Files selected for processing (21)
Dockerfile_bg_projects_refresh(1 hunks)backend/controllers/cache.go(2 hunks)backend/controllers/github.go(2 hunks)backend/models/storage.go(1 hunks)backend/service_clients/projects_service.go(1 hunks)backend/services/repos.go(2 hunks)backend/utils/bitbucket.go(2 hunks)backend/utils/github.go(0 hunks)backend/utils/github_test.go(3 hunks)backend/utils/gitlab.go(2 hunks)background/projects-refresh-service/.gitignore(1 hunks)background/projects-refresh-service/go.mod(1 hunks)background/projects-refresh-service/projects_refesh_main.go(1 hunks)ee/cli/pkg/policy/policy.go(2 hunks)ee/cli/pkg/utils/github.go(0 hunks)ee/drift/tasks/github.go(1 hunks)ee/drift/utils/github.go(1 hunks)fly-projects-refresh-service.toml(1 hunks)libs/git_utils/clone_utils.go(1 hunks)next/controllers/github.go(2 hunks)next/services/config.go(1 hunks)
💤 Files with no reviewable changes (2)
- ee/cli/pkg/utils/github.go
- backend/utils/github.go
🧰 Additional context used
🧠 Learnings (3)
backend/utils/bitbucket.go (1)
Learnt from: wenzel-felix
PR: diggerhq/digger#1933
File: libs/storage/azure_plan_storage.go:19-44
Timestamp: 2025-04-03T16:10:15.929Z
Learning: When handling errors from the Azure SDK for Go's blob storage, check for "404" or "not found" strings in the error message to detect missing blobs: `if strings.Contains(err.Error(), "404") || strings.Contains(strings.ToLower(err.Error()), "not found")`. This approach works reliably across different versions of the SDK.
backend/controllers/cache.go (1)
Learnt from: wenzel-felix
PR: diggerhq/digger#1933
File: libs/storage/azure_plan_storage.go:19-44
Timestamp: 2025-04-03T16:10:15.929Z
Learning: When handling errors from the Azure SDK for Go's blob storage, check for "404" or "not found" strings in the error message to detect missing blobs: `if strings.Contains(err.Error(), "404") || strings.Contains(strings.ToLower(err.Error()), "not found")`. This approach works reliably across different versions of the SDK.
backend/controllers/github.go (1)
Learnt from: wenzel-felix
PR: diggerhq/digger#1933
File: libs/storage/azure_plan_storage.go:19-44
Timestamp: 2025-04-03T16:10:15.929Z
Learning: When handling errors from the Azure SDK for Go's blob storage, check for "404" or "not found" strings in the error message to detect missing blobs: `if strings.Contains(err.Error(), "404") || strings.Contains(strings.ToLower(err.Error()), "not found")`. This approach works reliably across different versions of the SDK.
🧬 Code Graph Analysis (7)
backend/utils/github_test.go (1)
libs/git_utils/clone_utils.go (1)
CloneGitRepoAndDoAction(26-73)
backend/utils/bitbucket.go (1)
libs/git_utils/clone_utils.go (1)
CloneGitRepoAndDoAction(26-73)
backend/utils/gitlab.go (1)
libs/git_utils/clone_utils.go (1)
CloneGitRepoAndDoAction(26-73)
ee/cli/pkg/policy/policy.go (1)
libs/git_utils/clone_utils.go (1)
CloneGitRepoAndDoAction(26-73)
backend/controllers/cache.go (1)
libs/git_utils/clone_utils.go (1)
CloneGitRepoAndDoAction(26-73)
backend/controllers/github.go (1)
libs/git_utils/clone_utils.go (1)
CloneGitRepoAndDoAction(26-73)
next/controllers/github.go (1)
libs/git_utils/clone_utils.go (1)
CloneGitRepoAndDoAction(26-73)
🪛 golangci-lint (1.64.8)
background/projects-refresh-service/projects_refesh_main.go
64-64: slog: slog.Error arg "err" should be a string or a slog.Attr (possible missing key or value)
(govet)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Security Check
🔇 Additional comments (22)
backend/models/storage.go (1)
1645-1646: Good: extra context in debug logThe added
slog.Debugprovides useful visibility during project refresh and has negligible overhead at debug level.backend/controllers/github.go (2)
9-9: LGTM: Proper import for centralized Git utilitiesAdding the import for the new
git_utilspackage is correct and aligns with the refactoring to centralize Git cloning functionality.
919-919: LGTM: Function call correctly updated to use centralized Git utilitiesThe function call update maintains the same parameters while correctly referencing the new centralized
git_utils.CloneGitRepoAndDoAction. This refactoring improves code modularity and prepares for the new background service architecture.ee/drift/tasks/github.go (1)
8-8: LGTM: Import alias correctly updated to point to centralized Git utilitiesThe import alias change correctly points to the new
libs/git_utilspackage while maintaining the same alias name, ensuring no other code changes are needed in this file. This is a clean refactoring that supports the centralization effort.backend/utils/bitbucket.go (2)
5-5: LGTM: Import addition for centralized Git utilitiesThe import for
git_utilspackage is correctly added to support the refactored Git cloning functionality.
98-98: LGTM: Function call correctly updated to use centralized implementationThe function call properly references the new
git_utils.CloneGitRepoAndDoActionwhile maintaining the same parameters. This supports the modularization effort and takes advantage of the enhanced logging and error handling in the new implementation.backend/utils/gitlab.go (2)
5-5: LGTM: Import addition completes the consistent refactoring patternThe import for
git_utilspackage correctly supports the centralized Git cloning functionality, completing the consistent pattern across all Git provider utilities.
117-117: LGTM: Function call correctly updated with enhanced implementationThe function call properly uses
git_utils.CloneGitRepoAndDoActionwith the same parameters. This completes the refactoring effort to centralize Git operations and benefits from the enhanced logging, cleanup, and error handling in the new implementation.ee/drift/utils/github.go (1)
11-11: LGTM: Import refactored to use centralized git utilities.The import change correctly points to the new centralized
libs/git_utilspackage while maintaining the same alias, ensuring compatibility with existing function calls.next/controllers/github.go (2)
15-15: LGTM: Import added for centralized git utilities.The import addition correctly introduces the centralized
git_utilspackage.
542-542: LGTM: Function call updated to use centralized implementation.The function call correctly uses the new centralized
git_utils.CloneGitRepoAndDoActionwith compatible parameters.backend/controllers/cache.go (2)
5-5: LGTM: Import added for centralized git utilities.The import addition correctly introduces the centralized
git_utilspackage for the refactored cloning functionality.
71-71: LGTM: Function call updated to use centralized implementation.The asynchronous cache update correctly uses the new centralized
git_utils.CloneGitRepoAndDoActionfunction with compatible parameters.ee/cli/pkg/policy/policy.go (2)
4-4: LGTM: Import updated to use centralized git utilities.The import change correctly points to the new centralized
libs/git_utilspackage.
65-65: LGTM: Function call updated with correct parameters.The function call correctly uses the new centralized implementation with proper parameter mapping:
p.ManagementRepoUrlas repo URL"main"as branch- Empty string for commit hash (not needed)
p.GitTokenas token- Empty string for token username (not needed)
Dockerfile_bg_projects_refresh (2)
11-11: LGTM: Well-structured multi-stage build.The Dockerfile follows good practices with:
- Multi-stage build for smaller final image
- Version embedding via COMMIT_SHA
- Proper binary permissions
- Clear entrypoint definition
Also applies to: 24-25, 28-28
1-1: Base Image Go VersionGo 1.24.0 is available (released Feb 11, 2025), and the latest patch release is 1.24.4 (June 5, 2025). Consider updating your Dockerfile to:
- Pin to the exact patch:
FROM golang:1.24.4 AS builder- Or use the minor-version alias to automatically pick up patches:
FROM golang:1.24 AS builderThis ensures you get critical fixes while keeping your builds reproducible.
fly-projects-refresh-service.toml (1)
1-24: Configuration looks good for a background service.The Fly.io configuration is well-structured with appropriate resource allocation and auto-scaling settings for an on-demand background service.
background/projects-refresh-service/projects_refesh_main.go (1)
33-68: Well-structured service implementation.The main function correctly implements the project refresh workflow with proper error handling and logging throughout.
backend/services/repos.go (1)
44-50: Good refactoring to use external service.The refactoring successfully delegates the project refresh operation to the external service, simplifying the code and enabling asynchronous processing.
libs/git_utils/clone_utils.go (1)
26-73: Excellent centralization of git cloning logic.The
CloneGitRepoAndDoActionfunction is well-structured with comprehensive logging, proper error handling, and cleanup. This centralization eliminates code duplication across the codebase.backend/service_clients/projects_service.go (1)
42-107: Well-implemented service client.The
TriggerProjectsRefreshServicefunction is well-structured with proper error handling, logging, and machine configuration. The use of auto-destroy for cost efficiency is a good practice.
| return nil | ||
| }) | ||
| if err != nil { | ||
| slog.Error("error while cloning repo: %v", err) |
There was a problem hiding this comment.
Fix the slog.Error formatting.
The error argument is not properly formatted in the slog.Error call.
Apply this diff to fix the formatting:
- slog.Error("error while cloning repo: %v", err)
+ slog.Error("error while cloning repo", "error", err)📝 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.
| slog.Error("error while cloning repo: %v", err) | |
| slog.Error("error while cloning repo", "error", err) |
🧰 Tools
🪛 golangci-lint (1.64.8)
64-64: slog: slog.Error arg "err" should be a string or a slog.Attr (possible missing key or value)
(govet)
🤖 Prompt for AI Agents
In background/projects-refresh-service/projects_refesh_main.go at line 64, the
slog.Error call incorrectly formats the error argument by using a printf-style
format string. Replace the current call with one that passes the error as a
separate argument or uses structured logging fields, ensuring the error is
properly included in the log output without using a format string.
motatoes
changed the title
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores