-
Notifications
You must be signed in to change notification settings - Fork 540
fix: show correct breadcrumb label for SDK evaluations #4567
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
base: main
Are you sure you want to change the base?
Changes from all commits
6eaecdd
445fc3f
585c6ad
c9d112d
01a6e6d
1706e43
ad6e140
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import sendgrid | ||||||||||||||||||||||||||||||||||||||||||
| from sendgrid.helpers.mail import Mail | ||||||||||||||||||||||||||||||||||||||||||
| import smtplib | ||||||||||||||||||||||||||||||||||||||||||
| from email.mime.multipart import MIMEMultipart | ||||||||||||||||||||||||||||||||||||||||||
| from email.mime.text import MIMEText | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| from fastapi import HTTPException | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -10,16 +10,25 @@ | |||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| log = get_logger(__name__) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| # Initialize SendGrid only if enabled | ||||||||||||||||||||||||||||||||||||||||||
| if env.sendgrid.enabled: | ||||||||||||||||||||||||||||||||||||||||||
| sg = sendgrid.SendGridAPIClient(api_key=env.sendgrid.api_key) | ||||||||||||||||||||||||||||||||||||||||||
| log.info("✓ SendGrid enabled") | ||||||||||||||||||||||||||||||||||||||||||
| # Determine which email backend to use (SMTP > SendGrid > no-op) | ||||||||||||||||||||||||||||||||||||||||||
| _USE_SMTP = env.smtp.enabled | ||||||||||||||||||||||||||||||||||||||||||
| _USE_SENDGRID = not _USE_SMTP and env.sendgrid.enabled | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if _USE_SMTP: | ||||||||||||||||||||||||||||||||||||||||||
| log.info( | ||||||||||||||||||||||||||||||||||||||||||
| "✓ Email enabled via SMTP (%s:%s)", env.smtp.host, env.smtp.port | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| elif _USE_SENDGRID: | ||||||||||||||||||||||||||||||||||||||||||
| import sendgrid | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| _sg = sendgrid.SendGridAPIClient(api_key=env.sendgrid.api_key) | ||||||||||||||||||||||||||||||||||||||||||
| log.info("✓ Email enabled via SendGrid (legacy)") | ||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||
| sg = None | ||||||||||||||||||||||||||||||||||||||||||
| _sg = None | ||||||||||||||||||||||||||||||||||||||||||
| if env.sendgrid.api_key and not env.sendgrid.from_address: | ||||||||||||||||||||||||||||||||||||||||||
| log.warn("✗ SendGrid disabled: missing sender email address") | ||||||||||||||||||||||||||||||||||||||||||
| log.warn("✗ Email disabled: missing sender email address") | ||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||
| log.warn("✗ SendGrid disabled") | ||||||||||||||||||||||||||||||||||||||||||
| log.warn("✗ Email disabled") | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def read_email_template(template_file_path): | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -35,6 +44,46 @@ def read_email_template(template_file_path): | |||||||||||||||||||||||||||||||||||||||||
| return template_file.read() | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def _send_via_smtp(to_email: str, subject: str, html_content: str, from_email: str) -> None: | ||||||||||||||||||||||||||||||||||||||||||
| """Send email using SMTP.""" | ||||||||||||||||||||||||||||||||||||||||||
| msg = MIMEMultipart("alternative") | ||||||||||||||||||||||||||||||||||||||||||
| msg["Subject"] = subject | ||||||||||||||||||||||||||||||||||||||||||
| msg["From"] = from_email | ||||||||||||||||||||||||||||||||||||||||||
| msg["To"] = to_email | ||||||||||||||||||||||||||||||||||||||||||
| msg.attach(MIMEText(html_content, "html")) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| smtp_host = env.smtp.host | ||||||||||||||||||||||||||||||||||||||||||
| smtp_port = env.smtp.port | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if env.smtp.use_tls: | ||||||||||||||||||||||||||||||||||||||||||
| server = smtplib.SMTP(smtp_host, smtp_port) | ||||||||||||||||||||||||||||||||||||||||||
| server.ehlo() | ||||||||||||||||||||||||||||||||||||||||||
| server.starttls() | ||||||||||||||||||||||||||||||||||||||||||
| server.ehlo() | ||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||
| server = smtplib.SMTP(smtp_host, smtp_port) | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+59
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add an SMTP connection timeout to prevent request hangs.
Suggested fix- server = smtplib.SMTP(smtp_host, smtp_port)
+ server = smtplib.SMTP(smtp_host, smtp_port, timeout=10)
...
- server = smtplib.SMTP(smtp_host, smtp_port)
+ server = smtplib.SMTP(smtp_host, smtp_port, timeout=10)Also applies to: 67-69 |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||
| if env.smtp.username and env.smtp.password: | ||||||||||||||||||||||||||||||||||||||||||
| server.login(env.smtp.username, env.smtp.password) | ||||||||||||||||||||||||||||||||||||||||||
| server.sendmail(from_email, [to_email], msg.as_string()) | ||||||||||||||||||||||||||||||||||||||||||
| finally: | ||||||||||||||||||||||||||||||||||||||||||
| server.quit() | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def _send_via_sendgrid(to_email: str, subject: str, html_content: str, from_email: str) -> None: | ||||||||||||||||||||||||||||||||||||||||||
| """Send email using SendGrid (legacy fallback).""" | ||||||||||||||||||||||||||||||||||||||||||
| from sendgrid.helpers.mail import Mail | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| message = Mail( | ||||||||||||||||||||||||||||||||||||||||||
| from_email=from_email, | ||||||||||||||||||||||||||||||||||||||||||
| to_emails=to_email, | ||||||||||||||||||||||||||||||||||||||||||
| subject=subject, | ||||||||||||||||||||||||||||||||||||||||||
| html_content=html_content, | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| _sg.send(message) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| async def send_email( | ||||||||||||||||||||||||||||||||||||||||||
| to_email: str, subject: str, html_content: str, from_email: str | ||||||||||||||||||||||||||||||||||||||||||
| ) -> bool: | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -54,20 +103,15 @@ async def send_email( | |||||||||||||||||||||||||||||||||||||||||
| HTTPException: If there is an error sending the email. | ||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| # No-op if SendGrid is disabled | ||||||||||||||||||||||||||||||||||||||||||
| if not env.sendgrid.enabled: | ||||||||||||||||||||||||||||||||||||||||||
| log.info(f"[SENDGRID] Email disabled - would send '{subject}' to {to_email}") | ||||||||||||||||||||||||||||||||||||||||||
| if not _USE_SMTP and not _USE_SENDGRID: | ||||||||||||||||||||||||||||||||||||||||||
| log.info(f"[EMAIL] Email disabled - would send '{subject}' to {to_email}") | ||||||||||||||||||||||||||||||||||||||||||
| return True | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| message = Mail( | ||||||||||||||||||||||||||||||||||||||||||
| from_email=from_email, | ||||||||||||||||||||||||||||||||||||||||||
| to_emails=to_email, | ||||||||||||||||||||||||||||||||||||||||||
| subject=subject, | ||||||||||||||||||||||||||||||||||||||||||
| html_content=html_content, | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||
| sg.send(message) | ||||||||||||||||||||||||||||||||||||||||||
| if _USE_SMTP: | ||||||||||||||||||||||||||||||||||||||||||
| _send_via_smtp(to_email, subject, html_content, from_email) | ||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||
| _send_via_sendgrid(to_email, subject, html_content, from_email) | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+111
to
+114
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE="api/oss/src/services/email_service.py"
echo "== File = $FILE =="
wc -l "$FILE"
echo
# show the relevant portion around lines 90-150 (covers 111-114)
nl -ba "$FILE" | sed -n '80,160p'
# show definitions/usages of the helper functions to see if they are sync and how they perform network I/O
echo
echo "== Look for _send_via_smtp definition =="
rg -n "def _send_via_smtp" "$FILE"
echo
nl -ba "$FILE" | sed -n '1,240p' | sed -n '1,240p' | head -n 40
echo
echo "== Look for _send_via_sendgrid definition =="
rg -n "def _send_via_sendgrid" "$FILE"
echo
# print near each helper definition block (best-effort: show 40 lines after match)
python3 - <<'PY'
import re, itertools
path="api/oss/src/services/email_service.py"
lines=open(path,'r',encoding='utf-8').read().splitlines()
# find line numbers for defs
for name in ["_send_via_smtp","_send_via_sendgrid","send_email"]:
for i,l in enumerate(lines, start=1):
if re.search(rf"def {re.escape(name)}\b", l):
start=max(1,i-10); end=min(len(lines), i+120)
print(f"\n== {name} around line {i} (show {start}-{end}) ==")
for j in range(start,end+1):
print(f"{j:4d}: {lines[j-1]}")
break
PY
# run ruff (non-destructive) in api folder per guidelines to see if there are existing issues related to this code
echo
echo "== ruff format/check in api/ (targeting oss tree only if available) =="
ls api
ruff --version || true
ruff format api/oss/src/services/email_service.py
ruff check api/oss/src/services/email_service.py --fixRepository: Agenta-AI/agenta Length of output: 216 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE="api/oss/src/services/email_service.py"
echo "== File = $FILE =="
wc -l "$FILE"
echo
# Line-numbered view of the relevant range (avoid `nl` which isn't available)
python3 - <<'PY'
path="api/oss/src/services/email_service.py"
start=80
end=160
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i in range(start, min(end, len(lines))):
print(f"{i+1:4d}: {lines[i].rstrip()}")
PY
echo
echo "== Locate helper definitions =="
rg -n "def _send_via_smtp|def _send_via_sendgrid|async def send_email" "$FILE" || true
echo
echo "== Dump helper/send_email blocks (best-effort) =="
python3 - <<'PY'
import re
path="api/oss/src/services/email_service.py"
lines=open(path,'r',encoding='utf-8').read().splitlines()
def dump_at(name):
for idx,l in enumerate(lines, start=1):
if re.match(rf"\s*def {re.escape(name)}\b", l) or re.match(rf"\s*async def {re.escape(name)}\b", l):
start=max(1, idx-10); end=min(len(lines), idx+200)
print(f"\n== {name} starting at line {idx} (show {start}-{end}) ==")
for j in range(start,end+1):
print(f"{j:4d}: {lines[j-1]}")
return
print(f"\n== {name} not found ==")
for n in ["send_email","_send_via_smtp","_send_via_sendgrid"]:
dump_at(n)
PY
echo
echo "== Try ruff on this file (if ruff exists) =="
command -v ruff >/dev/null 2>&1 && ruff --version || echo "ruff not found in sandbox"
if command -v ruff >/dev/null 2>&1; then
ruff format "$FILE" || true
ruff check "$FILE" --fix || true
fiRepository: Agenta-AI/agenta Length of output: 8421 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE="api/oss/src/services/email_service.py"
python3 - <<'PY'
path="api/oss/src/services/email_service.py"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i,l in enumerate(lines[:80], start=1):
print(f"{i:4d}: {l.rstrip()}")
PYRepository: Agenta-AI/agenta Length of output: 2839 Offload SMTP/SendGrid blocking calls from async
Suggested fix+import asyncio
...
- if _USE_SMTP:
- _send_via_smtp(to_email, subject, html_content, from_email)
- else:
- _send_via_sendgrid(to_email, subject, html_content, from_email)
+ if _USE_SMTP:
+ await asyncio.to_thread(
+ _send_via_smtp,
+ to_email=to_email,
+ subject=subject,
+ html_content=html_content,
+ from_email=from_email,
+ )
+ else:
+ await asyncio.to_thread(
+ _send_via_sendgrid,
+ to_email=to_email,
+ subject=subject,
+ html_content=html_content,
+ from_email=from_email,
+ )📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
| return True | ||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||
| raise HTTPException(status_code=500, detail=str(e)) | ||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -922,12 +922,40 @@ def enabled(self) -> bool: | |||||
|
|
||||||
|
|
||||||
| # --------------------------------------------------------------------------- | ||||||
| # sendgrid | ||||||
| # smtp | ||||||
| # --------------------------------------------------------------------------- | ||||||
|
|
||||||
|
|
||||||
| class SmtpConfig(BaseModel): | ||||||
| """SMTP Email configuration""" | ||||||
|
|
||||||
| host: str | None = os.getenv("SMTP_HOST") | ||||||
| port: int = int(os.getenv("SMTP_PORT", "587")) | ||||||
| username: str | None = os.getenv("SMTP_USERNAME") | ||||||
| password: str | None = os.getenv("SMTP_PASSWORD") | ||||||
| from_address: str | None = ( | ||||||
| os.getenv("SMTP_FROM_ADDRESS") | ||||||
| or os.getenv("SENDGRID_FROM_ADDRESS") | ||||||
| or os.getenv("AGENTA_AUTHN_EMAIL_FROM") | ||||||
| or os.getenv("AGENTA_SEND_EMAIL_FROM_ADDRESS") | ||||||
| ) | ||||||
| use_tls: bool = os.getenv("SMTP_USE_TLS", "true").lower() in ("true", "1", "yes") | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use shared truthy parsing for
Suggested fix- use_tls: bool = os.getenv("SMTP_USE_TLS", "true").lower() in ("true", "1", "yes")
+ use_tls: bool = (os.getenv("SMTP_USE_TLS") or "true").lower() in _TRUTHY📝 Committable suggestion
Suggested change
|
||||||
|
|
||||||
| model_config = ConfigDict(extra="ignore") | ||||||
|
|
||||||
| @property | ||||||
| def enabled(self) -> bool: | ||||||
| """SMTP enabled if host and from address are present""" | ||||||
| return bool(self.host and self.from_address) | ||||||
|
|
||||||
|
|
||||||
| # --------------------------------------------------------------------------- | ||||||
| # sendgrid (legacy — kept for backwards compatibility) | ||||||
| # --------------------------------------------------------------------------- | ||||||
|
|
||||||
|
|
||||||
| class SendgridConfig(BaseModel): | ||||||
| """SendGrid Email configuration""" | ||||||
| """SendGrid Email configuration (legacy)""" | ||||||
|
|
||||||
| api_key: str | None = os.getenv("SENDGRID_API_KEY") | ||||||
| from_address: str | None = ( | ||||||
|
|
@@ -1037,15 +1065,9 @@ def email_method(self) -> str: | |||||
| if env.agenta.access.email_disabled: | ||||||
| return "" | ||||||
|
|
||||||
| sendgrid_enabled = bool( | ||||||
| os.getenv("SENDGRID_API_KEY") | ||||||
| and ( | ||||||
| os.getenv("SENDGRID_FROM_ADDRESS") | ||||||
| or os.getenv("AGENTA_AUTHN_EMAIL_FROM") | ||||||
| or os.getenv("AGENTA_SEND_EMAIL_FROM_ADDRESS") | ||||||
| ) | ||||||
| ) | ||||||
| return "otp" if sendgrid_enabled else "password" | ||||||
| # SMTP takes priority, then SendGrid fallback | ||||||
| email_configured = env.smtp.enabled or env.sendgrid.enabled | ||||||
| return "otp" if email_configured else "password" | ||||||
|
|
||||||
| @property | ||||||
| def email_enabled(self) -> bool: | ||||||
|
|
@@ -1101,6 +1123,7 @@ class EnvironSettings(BaseModel): | |||||
| posthog: PostHogConfig = PostHogConfig() | ||||||
| redis: RedisConfig = RedisConfig() | ||||||
| sendgrid: SendgridConfig = SendgridConfig() | ||||||
| smtp: SmtpConfig = SmtpConfig() | ||||||
| stripe: StripeConfig = StripeConfig() | ||||||
| supertokens: SuperTokensConfig = SuperTokensConfig() | ||||||
|
|
||||||
|
|
||||||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: Agenta-AI/agenta
Length of output: 321
🏁 Script executed:
Repository: Agenta-AI/agenta
Length of output: 12693
🏁 Script executed:
Repository: Agenta-AI/agenta
Length of output: 760
🏁 Script executed:
rg -n "sendgrid" api/uv.lockRepository: Agenta-AI/agenta
Length of output: 760
🏁 Script executed:
Repository: Agenta-AI/agenta
Length of output: 162
🏁 Script executed:
Repository: Agenta-AI/agenta
Length of output: 4745
Fix email_service network robustness (SMTP timeout, async blocking, formatting) and make SendGrid import safer.
api/oss/src/services/email_service.py:_send_via_smtpcreatessmtplib.SMTP(smtp_host, smtp_port)without a timeout (lines 59 & 64); addtimeout=...to avoid hangs.send_emailisasyncbut calls_send_via_smtp/_send_via_sendgridsynchronously (lines 113–116), blocking the event loop; run the send in a thread (e.g.,asyncio.to_thread) or use async clients._send_via_smtp,_send_via_sendgrid, andsend_emailuse positional parameters and positional calls (lines 45–47, 74–76, 89–91, 114–116); switch to*keyword-only params and format calls with#-grouped blocks.sendgridis present inapi/uv.lock(6.12.5), but defer import/client creation so enabling the config can’t break startup.