[Compose] Set Domain + CPU/Mem Fixes #199
Conversation
fix to match string in set domain
update cpu/mem for compose
WalkthroughThe pull request introduces significant changes to two files: Changes
Sequence DiagramsequenceDiagram
participant User
participant SetDomain
participant FileSystem
User->>SetDomain: Provide new domain
SetDomain->>FileSystem: Read .env file
FileSystem-->>SetDomain: Current project base URL
SetDomain->>SetDomain: Preserve existing path
SetDomain->>FileSystem: Update .env file
SetDomain->>FileSystem: Update MOTD file
SetDomain-->>User: Domain updated successfully
Possibly related PRs
Poem
✨ 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: 2
🧹 Nitpick comments (4)
hossted/setDomain.go (4)
114-119: Use%wfor better error wrapping.
Currently, the code uses%vfor wrapping the underlying error. Using%wis recommended for Go 1.13+ error chaining.- return fmt.Errorf("error reading PROJECT_BASE_URL: %v", err) + return fmt.Errorf("error reading PROJECT_BASE_URL: %w", err)
120-128: Remove commented-out code.
The commented line//newDomain := "new-domain.com"can be removed to improve readability.- //newDomain := "new-domain.com"
272-295: Handle potential whitespace around '='.
Users might add spacing around key-value pairs in the.envfile. Consider trimming whitespace to avoid parse issues.- if strings.HasPrefix(line, "PROJECT_BASE_URL=") { + trimmedLine := strings.TrimSpace(line) + if strings.HasPrefix(trimmedLine, "PROJECT_BASE_URL=") {
297-341: Consider ignoring case and handling multiple definitions.
If multiple or differently cased definitions ofPROJECT_BASE_URLexist, only the first one is replaced. Confirm that is desired or if a more robust approach is needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hossted/service/compose/reconcile_compose.go(1 hunks)hossted/setDomain.go(3 hunks)
🔇 Additional comments (4)
hossted/setDomain.go (3)
4-4: Import statement is used below, looks good.
129-132: Ensure consistent domain updates in MOTD.
If the MOTD file lacks the exact existing domain string, the update might be skipped. Consider adding a fallback logic if the domain isn't found.
342-380: Verify domain replacements with multiple references.
If the MOTD has multiple occurrences of the existing domain, the code replaces all of them. Ensure that is the intended behavior.hossted/service/compose/reconcile_compose.go (1)
633-641: Memory usage conversion is concise and clear.
Returning total memory in GB as a string is straightforward and suitable for logging or external reporting.
| // preservePath replaces the domain in a URL while preserving the path. | ||
| func preservePath(existingDomain, newDomain string) string { | ||
| // Check if the existing domain contains a path | ||
| if idx := strings.Index(existingDomain, "/"); idx != -1 { | ||
| // Extract the path and append it to the new domain | ||
| return newDomain + existingDomain[idx:] | ||
| } | ||
| // If no path exists, just return the new domain | ||
| return newDomain | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Schema or protocol considerations.
When the existingDomain includes "http://" or "https://", the slash detection might remove the protocol. Decide whether to preserve the scheme in addition to the path.
| // getCPUUsage returns the average CPU usage percentage and the number of CPU cores. | ||
| func getCPUUsage() (string, error) { | ||
| percentages, err := cpu.Percent(time.Second, false) | ||
|
|
||
| // Get the number of CPU cores | ||
| numCores, err := cpu.Counts(true) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| if len(percentages) > 0 { | ||
| return fmt.Sprintf("%.2f%%", percentages[0]), nil | ||
| } | ||
|
|
||
| return "", fmt.Errorf("no CPU usage data available") | ||
| // Convert the number of cores to a string | ||
| return fmt.Sprintf("%d", numCores), nil |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Correct doc comment to match returning the number of CPU cores.
The doc states it returns average CPU usage, but it strictly returns the number of CPU cores. Update the doc or rename the function for clarity.
-// getCPUUsage returns the average CPU usage percentage and the number of CPU cores.
+// getCPUUsage returns the number of CPU cores as a string.📝 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.
| // getCPUUsage returns the average CPU usage percentage and the number of CPU cores. | |
| func getCPUUsage() (string, error) { | |
| percentages, err := cpu.Percent(time.Second, false) | |
| // Get the number of CPU cores | |
| numCores, err := cpu.Counts(true) | |
| if err != nil { | |
| return "", err | |
| } | |
| if len(percentages) > 0 { | |
| return fmt.Sprintf("%.2f%%", percentages[0]), nil | |
| } | |
| return "", fmt.Errorf("no CPU usage data available") | |
| // Convert the number of cores to a string | |
| return fmt.Sprintf("%d", numCores), nil | |
| // getCPUUsage returns the number of CPU cores as a string. | |
| func getCPUUsage() (string, error) { | |
| // Get the number of CPU cores | |
| numCores, err := cpu.Counts(true) | |
| if err != nil { | |
| return "", err | |
| } | |
| // Convert the number of cores to a string | |
| return fmt.Sprintf("%d", numCores), nil |
Merge pull request #199 from hossted/dev
Summary by CodeRabbit
Release Notes
New Features
Changes
Improvements