style: update style for mcp server#8338
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/test-infra repository. |
| mcpServer.value.baseUrl = mcpServer.value.protocol + mcpServer.value.url; | ||
| if (mode.value == 'create') { | ||
| await createMcpServer(mcpServer.value); | ||
| MsgSuccess(i18n.global.t('commons.msg.createSuccess')); |
There was a problem hiding this comment.
There are two main changes:
-
Protocol Prepend: In the previous version, there was no protocol being prepended to
baseUrl. Now, it is mandatory by addingprotocolandurlfields in state object. -
Connection URL Handling: After fetching connection URL from API (
getMcpDomain()), the old approach of settingbaseUrldirectly withres.data.connUrlhas changed. New URLs will now also have their protocols extracted (either "http//" or "https//") and assigned separately toprotocolandurl, before assigning them back tobaseUrl.
Additionally, this might be affecting form validation rules as well, so you'd need review those too to ensure they account for these new properties.
| return res, nil | ||
| } | ||
| res.WebsiteID = website.ID | ||
| res.Domain = website.PrimaryDomain |
There was a problem hiding this comment.
The changes made in this code snippet seem appropriate since returning nil instead of an error indicates that there was no issue fetching the website record with the provided ID. However, for better readability and maintainability, it's good practice to log errors if they occur. Here's an optimized version:
func (m McpServerService) GetBindDomain() (response.McpBindDomainRes, error) {
var res response.McpBindDomainRes
websiteRepo := &WebsiteRepository{}
err := websiteRepo.GetFirst(commonRepo.WithByID(websiteID), &res)
if err != nil {
log.Errorf("Failed to fetch website with ID %d: %v", websiteID, err)
return res, err // Keeping original return pattern for clarity
}
res.WebsiteID = website.ID
res.Domain = website.PrimaryDomain
return res, nil
}This code includes logging when an error occurs using a package like log. Additionally, I've used type assertions in the repository call (&res) for explicitness, which might not be necessary in simpler contexts but can enhance clarity.
|
| _ = settingRepo.Update("MCP_WEBSITE_ID", "0") | ||
| } | ||
| tx.Commit() | ||
|
|
There was a problem hiding this comment.
The code has an issue where the parameter req.ID is compared to variable websiteID, which seems incorrect. Typically, you should compare them if they refer to different objects or IDs. However, given the context of the function name and usage, it might be intended to delete the website identified by req.ID. If so, ensure that websiteID is correctly set to match the value of req.ID.
Additionally, there's unnecessary use of _ before settingRepo.UpdateOrCreate("MCP_WEBSITE_ID", "0"). Removing this underscore would make the code cleaner.
Here’s a revised version with these adjustments:
@@ -498,7 +498,11 @@ func (w WebsiteService) DeleteWebsite(req request.WebsiteDelete) error {
}
websiteID := GetWebsiteID()
if req.ID == websiteID {
- _ = settingRepo.UpdateOrCreate("MCP_WEBSITE_ID", "0")
+ err := settingRepo.Update("MCP_WEBSITE_ID", "0")
+ if err != nil {
+ return err // Handle the error appropriately
+ }
}
tx.Commit()
Make sure to handle any potential errors from the Update method appropriately based on your application's requirements.
|
/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 |
3 participants
No description provided.