-
Notifications
You must be signed in to change notification settings - Fork 289
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
Interactive Message Response Size Handling #1263
Conversation
Signed-off-by: syedsadath-17 <[email protected]>
Signed-off-by: syedsadath-17 <[email protected]>
Signed-off-by: syedsadath-17 <[email protected]>
Signed-off-by: syedsadath-17 <[email protected]>
Signed-off-by: sadath-12 <[email protected]>
Message: api.Message{ | ||
PlaintextInputs: resp.PlaintextInputs, | ||
}, | ||
if strings.Contains(resp.Description, "logs") { |
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.
Why there is a condition for logs? I might be executing kubectl describe ...
and still I will need pods dropdown, and that means , describe
also should be included. Could you please take a look at this part again?
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.
You mean describe
should also give file instead ?
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.
No, maybe there is a misunderstanding. The primary motivation is to not send an attachment for the interactive blocks. Those blocks are the interactive dropdown elements e.g. namespace, name, verb . So, once you execute kubectl
it will show you interactive elements, so that elements should be limited so there wont be an attachment. More concrete example is, once you select namespace, it will return resources (e.g. pods), if those pods > 100 you will see an attachment in the slack UI, you need to prevent that.
Message: api.Message{ | ||
PlaintextInputs: resp.PlaintextInputs, | ||
}, | ||
if strings.Contains(resp.Description, "logs") { |
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.
Same as above
This PR was proposed filtering based on the commands but the idea seems to be with the parameters and many other things stepping down from this since haven't spent much time with codebase and its working |
Description
Changes proposed in this pull request:
If resources are request and their size exceeds the max limit of text allowed in slack instead of showing them in a file , we will limit the size to 45 now and only show 45 items maintaining the interactivity for the interactive messages , in the future we will be implementing pagination for this
Related issue(s)
Fixes: #1051