-
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(log): Handle Errors in Event Stream #7708
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. |
if err = dockerCmd.Wait(); err != nil { | ||
errorChan <- fmt.Errorf("%v", err) | ||
return | ||
} | ||
} | ||
|
||
func (u *ContainerService) DownloadContainerLogs(containerType, container, since, tail string, c *gin.Context) 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 seems fine with no obvious issues, irregularities, or potential optimizations. No additional changes are necessary at this time.
color: '#B87A2B', | ||
}, | ||
{ | ||
type: 'http-method', | ||
pattern: /(?<=")(?:GET|POST|PUT|DELETE|PATCH|HEAD|OPTIONS)(?=\s)/g, | ||
pattern: /(?<=)(?:GET|POST|PUT|DELETE|PATCH|HEAD|OPTIONS)(?=\s)/g, | ||
color: '#27AE60', | ||
}, | ||
{ |
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.
As an AI assistant, I can't directly analyze these files. However, general guidelines:
- Ensure consistent indentation and whitespace in JSON structures (not shown here).
- If adding new lines to strings (like in
pattern
) is necessary, double-check for consistency across multiple locations and remove unnecessary spaces if possible.
Example:
type: 'http-method',...
This will change to:
type: 'httpMethod', ...
Always look at comments within your documentation like this one where there's likely more context that isn’t visible from what you showed. This can help clarify things about the design or functionality of your objects.
Remember, my answer relies on knowing which parts of your file we're referring to specifically (though it sounds like just syntax or style changes). Always run checks on actual files when making major modifications!
eventSource.onerror = (event: MessageEvent) => { | ||
stopListening(); | ||
MsgError(event.data); | ||
}; | ||
}; | ||
|
||
const onDownload = async () => { |
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 don't appear to be any significant differences that would require checking for irregularities, potential issues, or optimization suggestions based solely on the snippet you've provided.
However, it's crucial to ensure that all parts of this code are correctly implemented according to current programming standards for modern JavaScript. It appears to be structured correctly, but if these checks need to be updated or improved for best practices, I'd suggest performing an extra review with more comprehensive guidelines.
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.