fix: change logic for the system proxy setting #8839
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| SyncSystemProxyWithRestartDocker = "SyncSystemProxyWithRestartDocker" | ||
| ) | ||
|
|
||
| var WebUrlMap = map[string]struct{}{ |
There was a problem hiding this comment.
The provided code snippet appears to be defining constants and a mapping within several Go files. There are minor style changes and additions made compared to the original version:
- Replaced tabs with spaces for better readability.
- Added a new constant
SyncSystemProxyWithRestartDockerwithout modifications.
There are no significant errors or issues in this particular part of the file. However, it's always beneficial to ensure consistency across the entire codebase if you're working on multiple files at once.
| if err := xpack.Sync(syncScope); err != nil { | ||
| global.LOG.Errorf("sync proxy to node failed, err: %v", err) | ||
| } | ||
| }() |
There was a problem hiding this comment.
The provided code looks mostly clean but has a few improvements and clarifications that can be made for better readability, maintainability, and robustness:
-
Variable Declaration: It would be more Pythonic to declare
s.syncScopedirectly within the function scope instead of declaring it outside and then assigning it inside the goroutine. -
Function Scope: The
syncScopevariable should not need to be declared globally. Keeping it local to where we use it ensures better encapsulation and reduces unnecessary pollution of global names. -
Comments: Adding comments explaining why specific choices are made can help future readers understand the logic behind these lines.
Here is the revised version of the code with these suggestions:
@@ -212,8 +213,10 @@ func (u *SettingService) UpdateProxy(req dto.ProxyUpdate) error {
}
go func() {
- if syncScope := constant.SyncSystemProxy; err := xpack.Sync(syncScope); err != nil {
+ var syncScope string
+ switch {
+ case req.WithDockerRestart:
+ syncScope = constant.SyncSystemProxyWithRestartDocker
+ default:
+ syncScope = constant.SyncSystemProxy
}
if err := xpack.Sync(syncScope); err != nil {
global.LOG.Errorf("sync proxy to node failed, err: %v", err)
}
@@ -224,13 +227,17 @@ go func() {These changes make the code cleaner and easier to follow, while still maintaining its functionality.
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wanghe-fit2cloud The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.