korbit-ai[bot] commented on code in PR #35652:
URL: https://github.com/apache/superset/pull/35652#discussion_r2430843634
##########
superset/reports/notifications/email.py:
##########
@@ -97,22 +97,41 @@ def _name(self) -> str:
def _get_smtp_domain() -> str:
return parseaddr(current_app.config["SMTP_MAIL_FROM"])[1].split("@")[1]
- def _error_template(self, text: str) -> str:
+ def _error_template(self, text: str, img_tag: str = "") -> str:
call_to_action = self._get_call_to_action()
return __(
"""
<p>Your report/alert was unable to be generated because of the
following error: %(text)s</p>
<p>Please check your dashboard/chart for errors.</p>
<p><b><a href="%(url)s">%(call_to_action)s</a></b></p>
+ %(screenshots)s
""", # noqa: E501
text=text,
url=self._content.url,
call_to_action=call_to_action,
+ screenshots=img_tag,
)
def _get_content(self) -> EmailContent:
if self._content.text:
- return EmailContent(body=self._error_template(self._content.text))
+ # Error case - include screenshots if available
+ images = {}
+ img_tag_str = ""
+ if self._content.screenshots:
+ domain = self._get_smtp_domain()
+ images = {
+ make_msgid(domain)[1:-1]: screenshot
+ for screenshot in self._content.screenshots
+ }
+ for msgid in images.keys():
+ img_tag_str += (
+ f'<div class="image"><img width="1000"
src="cid:{msgid}"></div>'
+ )
Review Comment:
### Inefficient string concatenation in loop <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
String concatenation in a loop creates O(nĀ²) time complexity due to string
immutability in Python, where each concatenation creates a new string object.
###### Why this matters
For large numbers of screenshots, this becomes increasingly inefficient as
each iteration must copy all previous characters, leading to poor scalability.
###### Suggested change ā *Feature Preview*
Use a list to collect strings and join them at the end for O(n) complexity:
```python
img_tag_parts = []
for msgid in images.keys():
img_tag_parts.append(
f'<div class="image"><img width="1000" src="cid:{msgid}"></div>'
)
img_tag_str = ''.join(img_tag_parts)
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/62e1fc39-9a83-474d-9b6f-d1d0eff90a87/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/62e1fc39-9a83-474d-9b6f-d1d0eff90a87?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/62e1fc39-9a83-474d-9b6f-d1d0eff90a87?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/62e1fc39-9a83-474d-9b6f-d1d0eff90a87?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/62e1fc39-9a83-474d-9b6f-d1d0eff90a87)
</details>
<sub>
š¬ Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:4cab8aa9-24dc-42b7-bd13-64718bf0e750 -->
[](4cab8aa9-24dc-42b7-bd13-64718bf0e750)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
