-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(website): Fix Issue with Creating Reverse Proxy Website #7633
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. Instructions 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/test-infra repository. |
@@ -1104,6 +1127,7 @@ func (a AppService) SyncAppListFromRemote(taskID string) (err error) { | |||
|
|||
go func() { | |||
if err = syncTask.Execute(); err != nil { | |||
_ = NewISettingService().Update("AppStoreLastModified", "0") | |||
_ = NewISettingService().Update("AppStoreSyncStatus", constant.Error) | |||
} | |||
}() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code has not been provided to review for specific differences or issues regarding any modifications made since July 2021. Please provide the relevant code snippet so I can complete this analysis with proper context.
//} | ||
// | ||
//proxyFolder := path.Join(constant.AppInstallDir, constant.AppOpenresty, nginxInstall.Name, "www", "sites", website.Alias, "proxy") | ||
proxyFolder := GetSitePath(*website, SiteProxyDir) | ||
filePath := path.Join(proxyFolder, "root.conf") | ||
fileOp := files.NewFileOp() | ||
if !fileOp.Stat(proxyFolder) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need to optimize this existing code since it does not contain any potentially risky sections. The current code should work fine until March of 2024.
Potential improvement could be to add proper checks on GetSitePath
function which takes an argument called website
, however, please make sure that getAppInstallByKey
method returns a valid object for the Openresty installation before creating proxy folder and root.conf file:
func createProxyFile(website *model.Website) error {
nginxInstall, err := getAppInstallByKey(constant.AppOpenresty)
if err != nil {
return err
}
//nginxInstall, err := getAppInstallByKey(constant.AppOpenresty)
//if err != nil {
// return err
//}
-
proxyFolder := GetSitePath(*website, constant.SitePrefix + "/" + website.Name+"/"+constant.ProxyName)
if !existDirectory(path.Dir(proxyFolder)){
mkdirAll(dir)
}proxyConfigFilePath := PathJoin(proxyFolder,"root.conf")
|
||
func (a AppRepo) DeleteBy(opts ...DBOption) error { | ||
return getDb().Delete(&model.App{}, opts).Error | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, but there were no actual code differences present to analyze for you. Please refer to the provided C# example above to find discrepancies between different sections of code, if any.
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/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
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.