🔧 Refactor: Optimize config API handling and logging#1940
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors config API handling by moving environment-specific logic from the API function to the caller. The changes restrict config API calls to dev mode only while attempting to make the file writing logic environment-agnostic.
Key Changes:
- Config API calls now only execute in dev bot instances via a guard condition in the caller
- File writing and diff notification logic removed from dev-only guard, making it unconditional
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/tasks/tasks.go | Added dev mode check to conditionally call GetConfigFromAPI only when running as dev bot |
| src/ei/ei_api.go | Removed dev mode check wrapping the config file writing goroutine, making it run unconditionally |
| go func() { | ||
| jsonData, err := json.MarshalIndent(configResponse, "", " ") | ||
| if err != nil { | ||
| log.Printf("Failed to marshal config response: %v", err) | ||
| return | ||
| } | ||
| // If the file exists, compare it to the new one to avoid unnecessary writes | ||
| existingData, err := os.ReadFile("ttbb-data/ei-config.json") | ||
| if err == nil { | ||
| if bytes.Equal(existingData, jsonData) { | ||
| // No changes, skip writing | ||
| return | ||
| } | ||
| // If the file exists, compare it to the new one to avoid unnecessary writes | ||
| existingData, err := os.ReadFile("ttbb-data/ei-config.json") | ||
| if err == nil { | ||
| if bytes.Equal(existingData, jsonData) { | ||
| // No changes, skip writing | ||
| return | ||
| } | ||
| } | ||
| // Files are different, if we have an existing file, I want a diff | ||
| if len(existingData) > 0 { | ||
| if patch, perr := jsondiff.Compare(existingData, jsonData); perr == nil { | ||
| if b, merr := json.MarshalIndent(patch, "", " "); merr == nil { | ||
| if strings.Contains(string(b), "ei_hatchery_custom") { | ||
| // If the diff contains the string "ei_hatchery_custom" | ||
| u, err := s.UserChannelCreate(config.AdminUserID) | ||
| if err != nil { | ||
| log.Printf("Failed to create user channel for admin: %v", err) | ||
| return | ||
| } | ||
| var data discordgo.MessageSend | ||
| data.Components = []discordgo.MessageComponent{ | ||
| discordgo.TextDisplay{ | ||
| Content: fmt.Sprintf("```diff\n%s\n```", string(b)), | ||
| }, | ||
| } | ||
| _, sendErr := s.ChannelMessageSendComplex(u.ID, &data) | ||
| if sendErr != nil { | ||
| log.Print(sendErr) | ||
| } | ||
| } | ||
| // Files are different, if we have an existing file, I want a diff | ||
| if len(existingData) > 0 { | ||
| if patch, perr := jsondiff.Compare(existingData, jsonData); perr == nil { | ||
| if b, merr := json.MarshalIndent(patch, "", " "); merr == nil { | ||
| if strings.Contains(string(b), "ei_hatchery_custom") { | ||
| // If the diff contains the string "ei_hatchery_custom" | ||
| u, err := s.UserChannelCreate(config.AdminUserID) | ||
| if err != nil { | ||
| log.Printf("Failed to create user channel for admin: %v", err) | ||
| return | ||
| } | ||
| var data discordgo.MessageSend | ||
| data.Components = []discordgo.MessageComponent{ | ||
| discordgo.TextDisplay{ | ||
| Content: fmt.Sprintf("```diff\n%s\n```", string(b)), | ||
| }, | ||
| } | ||
| _, sendErr := s.ChannelMessageSendComplex(u.ID, &data) | ||
| if sendErr != nil { | ||
| log.Print(sendErr) | ||
| } | ||
| } else { | ||
| log.Printf("Failed to marshal config diff; proceeding to write file: %v", merr) | ||
| } | ||
| } else { | ||
| log.Printf("Config diff failed; proceeding to write file: %v", perr) | ||
| log.Printf("Failed to marshal config diff; proceeding to write file: %v", merr) | ||
| } | ||
| } else { | ||
| log.Printf("Config diff failed; proceeding to write file: %v", perr) | ||
| } | ||
| } | ||
|
|
||
| _ = os.MkdirAll("ttbb-data", os.ModePerm) | ||
| err = os.WriteFile("ttbb-data/ei-config.json", jsonData, 0644) | ||
| if err != nil { | ||
| log.Printf("Failed to write config file: %v", err) | ||
| } | ||
| }() | ||
| } | ||
| _ = os.MkdirAll("ttbb-data", os.ModePerm) | ||
| err = os.WriteFile("ttbb-data/ei-config.json", jsonData, 0644) | ||
| if err != nil { | ||
| log.Printf("Failed to write config file: %v", err) | ||
| } | ||
| }() |
There was a problem hiding this comment.
The file writing and diff notification logic now runs unconditionally for all bot instances (dev and prod), but this appears inconsistent with the change in tasks.go where GetConfigFromAPI is now only called in dev mode. This means in production, the goroutine will never execute since GetConfigFromAPI won't be called at all. If the intent is to have this logic run in production as well, the condition should be added back inside the goroutine instead of preventing the entire GetConfigFromAPI call in tasks.go.
| if config.IsDevBot() { | ||
| ei.GetConfigFromAPI(s) | ||
| } |
There was a problem hiding this comment.
Restricting GetConfigFromAPI to only dev mode changes the production bot behavior - it will no longer fetch config data from the API or write the ei-config.json file when the reload contracts command is executed. This appears to be a breaking change that should be verified as intentional, as it may impact production monitoring capabilities.
| if config.IsDevBot() { | |
| ei.GetConfigFromAPI(s) | |
| } | |
| ei.GetConfigFromAPI(s) |
No description provided.