make loading projects from push configurable #2010
Conversation
WalkthroughThe Changes
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 (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 (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Added configurable project loading on push events through a new environment variable in the backend system.
- Added
DIGGER_LOAD_PROJECTS_ON_PUSHenvironment flag inbackend/controllers/github.goto control project loading behavior on push events - Enhanced logging in push event handler with specific messages for project loading status
Note: This feature requires explicit environment configuration to enable project loading on push.
1 file reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
backend/controllers/github.go
Outdated
| slog.Info("Loading projects from GitHub repo (push event)", "loadProjectsOnPush", loadProjectsOnPush, "ref", ref, "defaultBranch", defaultBranch) | ||
| err := services.LoadProjectsFromGithubRepo(gh, strconv.FormatInt(installationId, 10), repoFullName, repoOwner, repoName, cloneURL, defaultBranch) |
There was a problem hiding this comment.
logic: Log level 'Info' for loading projects is inconsistent. For diagnostics, the key operation event should have level 'Debug' according to custom rules
|
|
||
| if strings.HasSuffix(ref, defaultBranch) { | ||
| slog.Info("Loading projects from GitHub repo (push event)", "loadProjectsOnPush", loadProjectsOnPush, "ref", ref, "defaultBranch", defaultBranch) | ||
| err := services.LoadProjectsFromGithubRepo(gh, strconv.FormatInt(installationId, 10), repoFullName, repoOwner, repoName, cloneURL, defaultBranch) |
There was a problem hiding this comment.
style: Prefer 'prNumber' over plain 'ref' in logs for easier troubleshooting
|
✅ No security or compliance issues detected. Reviewed everything up to 41f9117. Security Overview
Detected Code Changes
Reply to this PR with |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/controllers/github.go (1)
411-411: Consider documenting the case-sensitive requirement.The environment variable check uses exact string matching with "true". This means "True", "TRUE", or "1" won't work. Consider documenting this requirement or using
strconv.ParseBool()for more flexible boolean parsing.Alternative implementation for more flexible boolean parsing:
-loadProjectsOnPush := os.Getenv("DIGGER_LOAD_PROJECTS_ON_PUSH") +loadProjectsOnPushStr := os.Getenv("DIGGER_LOAD_PROJECTS_ON_PUSH") +loadProjectsOnPush, _ := strconv.ParseBool(loadProjectsOnPushStr)Then use
if loadProjectsOnPush {instead of the string comparison.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/controllers/github.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: motatoes
PR: diggerhq/digger#1855
File: backend/controllers/github.go:768-774
Timestamp: 2024-12-17T12:34:41.545Z
Learning: In the Digger project, the `DIGGER_GENERATION_WEBHOOK_SECRET` environment variable is optional and may be an empty string, so validation for its presence is not required in the code.
backend/controllers/github.go (1)
Learnt from: motatoes
PR: diggerhq/digger#1855
File: backend/controllers/github.go:768-774
Timestamp: 2024-12-17T12:34:41.545Z
Learning: In the Digger project, the `DIGGER_GENERATION_WEBHOOK_SECRET` environment variable is optional and may be an empty string, so validation for its presence is not required in the code.
🧬 Code Graph Analysis (1)
backend/controllers/github.go (1)
backend/services/repos.go (1)
LoadProjectsFromGithubRepo(13-58)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Security Check
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
🔇 Additional comments (1)
backend/controllers/github.go (1)
413-424: Backwards compatibility consideration.This change introduces a breaking change where projects will no longer load on push events unless
DIGGER_LOAD_PROJECTS_ON_PUSH=trueis explicitly set. Ensure this is documented in the deployment/upgrade notes.The implementation correctly follows the PR objective of making project loading configurable. The error handling appropriately logs failures without stopping execution, and the logging provides good context for debugging.
we will need to make loading configurable (off for now) until we move it to a new service
Summary by CodeRabbit