Skip to content
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

fix(backend): correctly check if email service is set up #9497

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

ntindle
Copy link
Member

@ntindle ntindle commented Feb 18, 2025

I made a mistake in how we check if postmark exists

Changes 🏗️

  • adds a more explicit setting of postmark to none and extra checking to prevent its use if it isn’t set

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • I have no plan, it’s a simple logic bug so if it passes CI it’s good

@ntindle ntindle requested a review from a team as a code owner February 18, 2025 13:06
@ntindle ntindle requested review from Pwuts and Bentlybro and removed request for a team February 18, 2025 13:06
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Silent Failure

The email sending silently fails when Postmark is not configured. Consider raising an exception or returning a status to indicate the failure to the caller.

if not self.postmark:
    logger.warning("Email tried to send without postmark configured")
    return

@github-actions github-actions bot added the platform/backend AutoGPT Platform - Back end label Feb 18, 2025
Copy link

netlify bot commented Feb 18, 2025

Deploy Preview for auto-gpt-docs-dev canceled.

Name Link
🔨 Latest commit 91ab569
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs-dev/deploys/67b485c87a8ece0008d27a0e

Copy link

deepsource-io bot commented Feb 18, 2025

Here's the code health analysis summary for commits 43460b8..91ab569. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ SuccessView Check ↗
DeepSource Python LogoPython✅ Success
❗ 4 occurences introduced
View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copy link

netlify bot commented Feb 18, 2025

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 91ab569
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67b485c8ed0b1c000899c187

@Pwuts Pwuts changed the title fix(backend): correctly check if email exists fix(backend): correctly check if email service is set up Feb 18, 2025
Copy link
Member

@Pwuts Pwuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@ntindle ntindle added this pull request to the merge queue Feb 18, 2025
Merged via the queue into dev with commit dcbbe11 Feb 18, 2025
23 checks passed
@ntindle ntindle deleted the ntindle/fix-email-not-configured branch February 18, 2025 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants