feat: add SendGrid destination - #194
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
masukai
left a comment
There was a problem hiding this comment.
Thanks for the contribution @PFCAaron12! 🙌
A SendGrid destination is a great addition. I have a few items that need to be addressed before we can merge:
1. Config model is incomplete
SendGridDestinationConfig is missing fields that the implementation relies on: from_email, from_name, subject_template, body_template, list_ids. Currently the code uses hasattr() / getattr() to check for them at runtime, but drt uses Pydantic models as the source of truth — all fields should be declared explicitly.
See how DiscordDestinationConfig handles this in drt/config/models.py for reference.
2. Tests don't exercise the real implementation
FakeSendGridDestination overrides load() entirely, so the actual SendGridDestination.load() is never called. Tests should mock httpx.Client (like tests/unit/test_discord.py does) rather than replacing the destination class.
3. CLA
Please sign the CLA by posting a comment:
I have read the CLA Document and I hereby sign the CLA
4. Minor
- Missing newline at end of
sendgrid.pyandtest_sendgrid.py
Happy to answer any questions — looking forward to the next revision! 🚀
|
Apologies for the delay. I’m away for a few days but will prioritize this when I’m back. |
|
I have made the changes. |
|
I have read the CLA Document and I hereby sign the CLA. |
masukai
left a comment
There was a problem hiding this comment.
Thanks for the quick revisions, @PFCAaron12 — and for submitting both this and #195! Really appreciate you helping expand drt's destination coverage at this early stage. 🙌
The config model rewrite and test improvements are solid — much better than the first version.
A few things I noticed that I'll clean up post-merge (no action needed from your side):
- Import sorting —
ruff check --fixwill sort these automatically. Tip: runningmake fmtlocally before pushing catches these. - Trailing newline at end of files — most editors can add this automatically (e.g., VS Code:
files.insertFinalNewline) resolve_env()helper — the codebase has a shared helper for resolving auth tokens from env vars (drt/config/credentials.py). I'll swap in the standard pattern.authfield type — I'll narrow it toBearerAuthfor fail-fast validation, and a few other minor consistency tweaks.
Merging with squash — thanks again for the contribution! 🎉
If you enjoy using drt, a ⭐ on the repo would mean a lot. Happy to see more PRs from you!
- Use resolve_env() for auth token resolution - Narrow auth field to BearerAuth (fail-fast validation) - Add describe() method to SendGridDestinationConfig - Fix load() signature to use DestinationConfig (Protocol compliance) - Add on_error="fail" break support within batch loop - Use sync_options.retry with _DEFAULT_RETRY fallback - Use json.dumps(row, default=str) for safe record preview - Remove list_ids from mail/send payload (not supported by API) - Use explicit default arg in do_post() closure - Add CHANGELOG entry - Fix trailing newlines, import ordering (ruff) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
c499614 to
45ef409
Compare
What does this PR do?
Add SendGridDestinationConfig to drt/config/models.py
Create drt/destinations/sendgrid.py
Register the new config in DestinationConfig union in models.py
Wire it in drt/cli/main.py _get_destination()
Add tests in tests/unit/test_sendgrid.py using a FakeSendGridDestination
Related Issue
Closes #36
Checklist
make test)make lint)CHANGELOG.md(if user-facing change)