-
Notifications
You must be signed in to change notification settings - Fork 8
Fix issue #52: Update README.md using new guideline #55
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
Conversation
WalkthroughReplaced a single consolidated sending example with six focused sending and batch examples; updated README to environment-based sandbox/production usage; removed several model enums in favor of plain strings, added optional message-related fields to Suppression, and updated tests to match these model changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Example as Example script
participant Factory as get_client()
participant Client as mt.MailtrapClient
participant API as Mailtrap API
Note over Example,Factory: Environment-based client selection (default | bulk | sandbox)
Example->>Factory: get_client(type_)
Factory-->>Client: configured client
Note over Example,Client: Send / Batch send flow
Example->>Client: send(mail) or batch_send(payload)
Client->>API: HTTP request (send / batch_send)
API-->>Client: response (SEND_ENDPOINT_RESPONSE / BATCH_SEND_ENDPOINT_RESPONSE)
Client-->>Example: response returned
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-27T18:15:36.126ZApplied to files:
🧬 Code graph analysis (1)examples/sending/batch_minimal_sending.py (3)
🪛 Ruff (0.14.1)examples/sending/batch_minimal_sending.py4-4: Possible hardcoded password assigned to: "API_TOKEN" (S105) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 4
🧹 Nitpick comments (22)
mailtrap/models/contacts.py (1)
86-86: Enum-to-string conversion looks correct.The transition from enum types (
ContactStatus,ContactImportStatus) to plainstrsimplifies the API but removes compile-time validation of valid status values. Consider documenting the valid status strings (e.g., "subscribed", "started", "finished") in docstrings or external documentation to help API consumers.Also applies to: 99-99
examples/sending/minimal_sending.py (4)
3-3: Use env var for secrets; avoid hardcoded tokens.Read API token from MAILTRAP_API_TOKEN to satisfy S105 and best practices.
+import os import mailtrap as mt -API_TOKEN = "<YOUR_API_TOKEN>" +API_TOKEN = os.environ["MAILTRAP_API_TOKEN"]
12-12: Fix type annotation for get_client.type_ is a string constant; annotate with Literal for correctness.
+from typing import Literal - -def get_client(type_: SendingType) -> mt.MailtrapClient: +def get_client(type_: Literal["default", "bulk", "sandbox"]) -> mt.MailtrapClient:
18-20: Get SANDBOX inbox_id from env, not a placeholder.- return mt.MailtrapClient( - token=API_TOKEN, sandbox=True, inbox_id="<YOUR_INBOX_ID>" - ) + return mt.MailtrapClient( + token=API_TOKEN, + sandbox=True, + inbox_id=os.environ["MAILTRAP_INBOX_ID"], + )
6-21: Reduce duplication across examples.SendingType and get_client repeat in multiple files. Consider extracting to examples/utils.py and importing in examples to DRY.
examples/sending/batch_minimal_sending.py (3)
3-3: Use MAILTRAP_API_TOKEN from env.Avoid embedding secrets, even placeholders.
+import os import mailtrap as mt -API_TOKEN = "<YOUR_API_TOKEN>" +API_TOKEN = os.environ["MAILTRAP_API_TOKEN"]
12-12: Correct get_client annotation.Use Literal to match actual string usage.
+from typing import Literal - -def get_client(type_: SendingType) -> mt.MailtrapClient: +def get_client(type_: Literal["default", "bulk", "sandbox"]) -> mt.MailtrapClient:
18-20: Read SANDBOX inbox_id from env.- return mt.MailtrapClient( - token=API_TOKEN, sandbox=True, inbox_id="<YOUR_INBOX_ID>" - ) + return mt.MailtrapClient( + token=API_TOKEN, + sandbox=True, + inbox_id=os.environ["MAILTRAP_INBOX_ID"], + )examples/sending/batch_advanced_sending.py (4)
6-6: Read token from MAILTRAP_API_TOKEN.Prevents accidental secret leaks and satisfies S105.
+import os import mailtrap as mt -API_TOKEN = "<YOUR_API_TOKEN>" +API_TOKEN = os.environ["MAILTRAP_API_TOKEN"]
15-15: Annotate get_client with Literal strings.Matches the string constants used.
+from typing import Literal - -def get_client(type_: SendingType) -> mt.MailtrapClient: +def get_client(type_: Literal["default", "bulk", "sandbox"]) -> mt.MailtrapClient:
28-31: Avoid import-time file I/O; build payload lazily.Reading welcome.png at import time raises on import if file is missing and slows module import. Build within a function or under main.
# Example refactor (outside this hunk): def build_batch_mail() -> mt.BatchSendEmailParams: welcome_image = Path(__file__).parent.joinpath("welcome.png").read_bytes() return mt.BatchSendEmailParams( base=mt.BatchMail( # ...same as now... attachments=[ mt.Attachment( content=base64.b64encode(welcome_image).decode("ascii"), filename="welcome.png", disposition=mt.Disposition.INLINE, mimetype="image/png", content_id="welcome.png", ) ], ), requests=[ ... ], ) if __name__ == "__main__": client = get_client(SendingType.DEFAULT) print(batch_send(client, build_batch_mail()))
71-71: Encode attachment content as str, not bytes.base64.b64encode returns bytes; many serializers expect a string. Decode to ASCII.
- content=base64.b64encode(welcome_image), + content=base64.b64encode(welcome_image).decode("ascii"),Please confirm Attachment.content expects a base64 string in the current SDK; adjust if bytes are explicitly supported.
examples/sending/batch_sending_with_template.py (4)
3-3: Use MAILTRAP_API_TOKEN env var.Avoid placeholder secrets in code.
+import os import mailtrap as mt -API_TOKEN = "<YOUR_API_TOKEN>" +API_TOKEN = os.environ["MAILTRAP_API_TOKEN"]
12-12: Literal-annotate get_client.+from typing import Literal - -def get_client(type_: SendingType) -> mt.MailtrapClient: +def get_client(type_: Literal["default", "bulk", "sandbox"]) -> mt.MailtrapClient:
18-20: Inbox id from env for SANDBOX.- return mt.MailtrapClient( - token=API_TOKEN, sandbox=True, inbox_id="<YOUR_INBOX_ID>" - ) + return mt.MailtrapClient( + token=API_TOKEN, + sandbox=True, + inbox_id=os.environ["MAILTRAP_INBOX_ID"], + )
6-21: Centralize SendingType/get_client to reduce duplication.Extract to a shared helper (examples/utils.py) and import across examples.
examples/sending/sending_with_template.py (4)
3-3: Read MAILTRAP_API_TOKEN from env.Prevents committing secrets and aligns with guidelines.
+import os import mailtrap as mt -API_TOKEN = "<YOUR_API_TOKEN>" +API_TOKEN = os.environ["MAILTRAP_API_TOKEN"]
12-12: Update get_client type annotation.Use Literal to reflect string values.
+from typing import Literal - -def get_client(type_: SendingType) -> mt.MailtrapClient: +def get_client(type_: Literal["default", "bulk", "sandbox"]) -> mt.MailtrapClient:
18-20: Env var for SANDBOX inbox_id.- return mt.MailtrapClient( - token=API_TOKEN, sandbox=True, inbox_id="<YOUR_INBOX_ID>" - ) + return mt.MailtrapClient( + token=API_TOKEN, + sandbox=True, + inbox_id=os.environ["MAILTRAP_INBOX_ID"], + )
6-21: Consider deduplication of SendingType/get_client.Extract once and import into examples.
README.md (1)
30-48: Consider aligning with the environment-based pattern introduced later.While this minimal example works, it hardcodes the API token. The subsequent "Sandbox vs Production" section (lines 50-87) demonstrates a more production-ready pattern using environment variables. Consider adding a note directing users to that section or using
os.environ.get()here as well for consistency.examples/sending/advanced_sending.py (1)
82-86: Consider whether this wrapper function adds value.The
sendfunction is a thin wrapper that simply delegates toclient.send(mail). While it might serve as a customization point, for an example file it could be seen as unnecessary indirection. However, if this pattern is used consistently across all example files for uniformity, it's acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
README.md(4 hunks)examples/sending.py(0 hunks)examples/sending/advanced_sending.py(1 hunks)examples/sending/batch_advanced_sending.py(1 hunks)examples/sending/batch_minimal_sending.py(1 hunks)examples/sending/batch_sending_with_template.py(1 hunks)examples/sending/minimal_sending.py(1 hunks)examples/sending/sending_with_template.py(1 hunks)mailtrap/models/contacts.py(2 hunks)mailtrap/models/messages.py(2 hunks)mailtrap/models/suppressions.py(1 hunks)tests/unit/api/contacts/test_contact_imports.py(3 hunks)tests/unit/api/contacts/test_contacts.py(3 hunks)tests/unit/api/suppressions/test_suppressions.py(1 hunks)tests/unit/api/testing/test_messages.py(1 hunks)
💤 Files with no reviewable changes (1)
- examples/sending.py
🧰 Additional context used
🧬 Code graph analysis (6)
examples/sending/sending_with_template.py (3)
mailtrap/client.py (1)
MailtrapClient(31-170)mailtrap/models/mail/mail.py (2)
MailFromTemplate(33-35)BaseMail(13-21)mailtrap/models/mail/address.py (1)
Address(9-11)
examples/sending/batch_advanced_sending.py (4)
mailtrap/client.py (2)
MailtrapClient(31-170)headers(137-144)mailtrap/models/mail/batch_mail.py (3)
BatchSendEmailParams(53-55)BatchMail(23-27)BatchEmailRequest(37-49)mailtrap/models/mail/address.py (1)
Address(9-11)mailtrap/models/mail/attachment.py (1)
Disposition(12-14)
examples/sending/batch_minimal_sending.py (3)
mailtrap/client.py (1)
MailtrapClient(31-170)mailtrap/models/mail/batch_mail.py (3)
BatchSendEmailParams(53-55)BatchMail(23-27)BatchEmailRequest(37-49)mailtrap/models/mail/address.py (1)
Address(9-11)
examples/sending/batch_sending_with_template.py (3)
mailtrap/client.py (1)
MailtrapClient(31-170)mailtrap/models/mail/batch_mail.py (3)
BatchSendEmailParams(53-55)BatchMailFromTemplate(31-33)BatchEmailRequest(37-49)mailtrap/models/mail/address.py (1)
Address(9-11)
examples/sending/minimal_sending.py (3)
mailtrap/client.py (1)
MailtrapClient(31-170)mailtrap/models/mail/mail.py (2)
BaseMail(13-21)mailtrap/models/mail/address.py (1)
Address(9-11)
examples/sending/advanced_sending.py (4)
mailtrap/client.py (2)
MailtrapClient(31-170)headers(137-144)mailtrap/models/mail/mail.py (2)
BaseMail(13-21)mailtrap/models/mail/address.py (1)
Address(9-11)mailtrap/models/mail/attachment.py (1)
Disposition(12-14)
🪛 LanguageTool
README.md
[grammar] ~188-~188: Ensure spelling is correct
Context: ...al_sending.py) - Batch sending advanced - [Bathc sending using template](examples/sendin...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
README.md
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.1)
examples/sending/sending_with_template.py
3-3: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/sending/batch_advanced_sending.py
6-6: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/sending/batch_minimal_sending.py
3-3: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/sending/batch_sending_with_template.py
3-3: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/sending/minimal_sending.py
3-3: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/sending/advanced_sending.py
6-6: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
🔇 Additional comments (13)
mailtrap/models/suppressions.py (1)
11-11: LGTM!The enum-to-string conversion for
typeandsending_streamfields is consistent with the broader refactoring. The changes are correct and align with the updated test expectations.Also applies to: 14-14
tests/unit/api/contacts/test_contact_imports.py (1)
137-137: LGTM!Test assertions correctly updated to compare against string literals, matching the model changes from enum to string types.
Also applies to: 192-192, 210-210
tests/unit/api/testing/test_messages.py (1)
546-546: LGTM!The test correctly compares
result.statusdirectly to the string "success", reflecting the change from enum to string type.tests/unit/api/suppressions/test_suppressions.py (1)
169-169: LGTM!Test assertions properly updated to use string literals for
typeandsending_streamfields, aligning with the model refactoring.Also applies to: 171-171
mailtrap/models/messages.py (1)
21-21: LGTM!The enum-to-string conversion for
resultandstatusfields is consistent with the refactoring applied across other model files. Changes are correct and tests have been updated accordingly.Also applies to: 115-115
tests/unit/api/contacts/test_contacts.py (1)
124-124: LGTM!All test assertions correctly updated to compare
contact.statusagainst the string literal "subscribed", matching the model change from enum to string type.Also applies to: 212-212, 299-299
README.md (2)
50-95: Excellent addition of environment-based configuration guidance.The new sandbox vs production section effectively demonstrates how to switch between environments using environment variables, which is a best practice for configuration management. The recommendations about separate tokens and centralized initialization are valuable.
177-216: Well-organized navigation for usage examples.The comprehensive categorization of examples by API type (Sending, Batch Sending, Sandbox, Contacts, etc.) makes it easy for users to find relevant code samples. This is a significant improvement in documentation structure.
examples/sending/advanced_sending.py (5)
6-6: Hardcoded API token is appropriate for an example file.The static analysis warning about a hardcoded password is a false positive in this context. Example files are expected to use placeholder tokens like
"<YOUR_API_TOKEN>"to guide users. The value clearly indicates where users should substitute their actual token.
9-13: SendingType as a simple class with string constants is appropriate.This pattern aligns with the PR objective to replace enums with simple strings. The approach is clear and avoids unnecessary complexity for example code.
28-29: File dependency is appropriately documented.The comment on line 28 clearly informs users that
welcome.pngshould exist in the same directory as the script. This is good documentation for example code.
31-79: Comprehensive demonstration of advanced email features.The example effectively showcases:
- Multiple recipient types (to, cc, bcc)
- Rich HTML content with inline images
- Base64-encoded inline attachments with proper content-id references
- Custom headers and variables
- Category tagging
This is an excellent reference for users needing to implement complex email functionality.
89-91: Main block demonstrates clear usage pattern.The
if __name__ == "__main__"block shows users exactly how to instantiate the client and send the email, making this example immediately runnable (after substituting placeholders).
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.
Actionable comments posted: 0
♻️ Duplicate comments (9)
examples/sending/batch_minimal_sending.py (1)
6-22: Consider extracting duplicated code to a shared utility module.Same duplication concern as in minimal_sending.py.
examples/sending/sending_with_template.py (1)
6-22: Consider extracting duplicated code to a shared utility module.Same duplication concern as in previous examples.
examples/sending/batch_advanced_sending.py (1)
9-27: Consider extracting duplicated code to a shared utility module.Same duplication concern as in previous examples.
examples/sending/batch_sending_with_template.py (1)
6-22: Consider extracting duplicated code to a shared utility module.Same duplication concern as in previous examples.
README.md (3)
57-61: Language identifier added.The past review comment about adding a language identifier to the code block has been addressed. The block now correctly uses
bashidentifier.
189-189: Typo fixed.The past review comment about fixing the typo "Bathc" → "Batch" has been addressed. Line 189 now correctly reads "Batch sending using template".
264-264: Git clone command added.The past review comment about adding the missing
git clonecommand has been addressed. Line 264 now includes the complete command.examples/sending/advanced_sending.py (2)
15-27: Error handling added.The past review comment about adding explicit handling for invalid
type_values has been addressed. The function now includes anelseclause (lines 26-27) that raises aValueErrorwith a descriptive message.
9-27: Consider extracting duplicated code to a shared utility module.Same duplication concern as in the other example files.
🧹 Nitpick comments (1)
examples/sending/minimal_sending.py (1)
6-22: Consider extracting duplicated code to a shared utility module.The
SendingTypeclass andget_clientfunction are duplicated across all six example files (minimal_sending.py, batch_minimal_sending.py, sending_with_template.py, batch_advanced_sending.py, batch_sending_with_template.py, and advanced_sending.py). While duplication in examples can aid standalone usage, a shared utility module (e.g.,examples/sending/utils.py) would reduce maintenance overhead and ensure consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(4 hunks)examples/sending/advanced_sending.py(1 hunks)examples/sending/batch_advanced_sending.py(1 hunks)examples/sending/batch_minimal_sending.py(1 hunks)examples/sending/batch_sending_with_template.py(1 hunks)examples/sending/minimal_sending.py(1 hunks)examples/sending/sending_with_template.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
examples/sending/advanced_sending.py (4)
mailtrap/client.py (2)
MailtrapClient(31-170)headers(137-144)mailtrap/models/mail/mail.py (2)
BaseMail(13-21)mailtrap/models/mail/address.py (1)
Address(9-11)mailtrap/models/mail/attachment.py (1)
Disposition(12-14)
examples/sending/sending_with_template.py (3)
mailtrap/client.py (1)
MailtrapClient(31-170)mailtrap/models/mail/mail.py (2)
MailFromTemplate(33-35)BaseMail(13-21)mailtrap/models/mail/address.py (1)
Address(9-11)
examples/sending/batch_advanced_sending.py (4)
mailtrap/client.py (2)
MailtrapClient(31-170)headers(137-144)mailtrap/models/mail/batch_mail.py (3)
BatchSendEmailParams(53-55)BatchMail(23-27)BatchEmailRequest(37-49)mailtrap/models/mail/address.py (1)
Address(9-11)mailtrap/models/mail/attachment.py (1)
Disposition(12-14)
examples/sending/batch_sending_with_template.py (3)
mailtrap/client.py (1)
MailtrapClient(31-170)mailtrap/models/mail/batch_mail.py (3)
BatchSendEmailParams(53-55)BatchMailFromTemplate(31-33)BatchEmailRequest(37-49)mailtrap/models/mail/address.py (1)
Address(9-11)
examples/sending/minimal_sending.py (3)
mailtrap/client.py (1)
MailtrapClient(31-170)mailtrap/models/mail/mail.py (2)
BaseMail(13-21)mailtrap/models/mail/address.py (1)
Address(9-11)
examples/sending/batch_minimal_sending.py (3)
mailtrap/client.py (1)
MailtrapClient(31-170)mailtrap/models/mail/batch_mail.py (3)
BatchSendEmailParams(53-55)BatchMail(23-27)BatchEmailRequest(37-49)mailtrap/models/mail/address.py (1)
Address(9-11)
🪛 Ruff (0.14.1)
examples/sending/advanced_sending.py
6-6: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
27-27: Avoid specifying long messages outside the exception class
(TRY003)
examples/sending/sending_with_template.py
3-3: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
22-22: Avoid specifying long messages outside the exception class
(TRY003)
examples/sending/batch_advanced_sending.py
6-6: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
27-27: Avoid specifying long messages outside the exception class
(TRY003)
examples/sending/batch_sending_with_template.py
3-3: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
22-22: Avoid specifying long messages outside the exception class
(TRY003)
examples/sending/minimal_sending.py
3-3: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
22-22: Avoid specifying long messages outside the exception class
(TRY003)
examples/sending/batch_minimal_sending.py
3-3: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
22-22: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test python3.9 on macos-latest
- GitHub Check: Test python3.12 on windows-latest
- GitHub Check: Test python3.13 on windows-latest
- GitHub Check: Test python3.9 on windows-latest
- GitHub Check: Test python3.10 on windows-latest
- GitHub Check: Test python3.11 on windows-latest
🔇 Additional comments (9)
examples/sending/minimal_sending.py (1)
25-39: LGTM!The example demonstrates the minimal sending workflow clearly with appropriate mail construction and client usage.
examples/sending/batch_minimal_sending.py (1)
25-49: LGTM!The batch sending example correctly demonstrates the minimal batch workflow with proper
BatchSendEmailParamsstructure.examples/sending/sending_with_template.py (1)
25-46: LGTM!The template-based sending example correctly demonstrates
MailFromTemplateusage with template UUID and variables.examples/sending/batch_advanced_sending.py (1)
30-100: LGTM!The advanced batch sending example effectively demonstrates inline attachments with proper base64 encoding, content-id referencing, and comprehensive mail configuration including CC, BCC, headers, and custom variables.
examples/sending/batch_sending_with_template.py (1)
25-58: LGTM!The batch template sending example correctly demonstrates
BatchMailFromTemplateusage with template UUID, variables, and per-request customization.README.md (3)
30-95: LGTM!The expanded usage section with environment-based configuration and sandbox/production switching is clear and helpful. The examples effectively demonstrate token management and client initialization patterns.
177-216: LGTM!The comprehensive "All usage examples" section provides excellent navigation to all available examples, well-organized by API category (Sending, Batch Sending, Sandbox, Contacts, Email Templates, Suppressions, General).
217-254: LGTM!The new "Supported functionality" section provides a clear, comprehensive overview of all SDK capabilities organized by functional area. This is valuable for users to quickly understand what the SDK can do.
examples/sending/advanced_sending.py (1)
30-93: LGTM!The advanced sending example effectively demonstrates comprehensive email functionality including inline attachments, HTML content with proper content-id referencing, custom headers, and custom variables.
| else: | ||
| raise ValueError(f"Invalid sending type: {type_}") |
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.
redundant else
| else: | |
| raise ValueError(f"Invalid sending type: {type_}") | |
| raise ValueError(f"Invalid sending type: {type_}") |
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.
Done
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 removed raise ValueError(f"Invalid sending type: {type_}") line at all but I meant to remove only else. It's redundant because all if branches above use return
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.
Ok. I will change it in the next PR, it will be related to examples.
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.
Actionable comments posted: 5
♻️ Duplicate comments (7)
examples/sending/sending_with_template.py (1)
6-9: Consider extracting SendingType to a shared module.This class is duplicated across all example files. Extracting to a shared module would reduce duplication.
examples/sending/minimal_sending.py (1)
7-10: Consider extracting SendingType to a shared module.This class is duplicated across all example files. Extracting to a shared module would reduce duplication.
examples/sending/batch_sending_with_template.py (1)
6-9: Consider extracting SendingType to a shared module.This class is duplicated across all example files. Extracting to a shared module would reduce duplication.
examples/sending/batch_minimal_sending.py (1)
7-10: Consider extracting SendingType to a shared module.This class is duplicated across all example files. Extracting to a shared module would reduce duplication.
examples/sending/advanced_sending.py (2)
9-12: Consider extracting SendingType to a shared module.This class is duplicated across all example files. Extracting to a shared module would reduce duplication.
15-26: Add error handling for invalid SendingType values (unresolved from past review).Past review comments indicate this was addressed in commit 1fb507a, but the current code doesn't include the fix. The function will return
Noneiftype_doesn't match any expected value.Apply this diff as previously suggested:
def get_client(type_: SendingType) -> mt.MailtrapClient: if type_ == SendingType.DEFAULT: return mt.MailtrapClient(token=API_TOKEN) elif type_ == SendingType.BULK: return mt.MailtrapClient(token=API_TOKEN, bulk=True) elif type_ == SendingType.SANDBOX: return mt.MailtrapClient( token=API_TOKEN, sandbox=True, inbox_id="<YOUR_INBOX_ID>", ) + else: + raise ValueError(f"Invalid sending type: {type_}")examples/sending/batch_advanced_sending.py (1)
15-26: Add error handling for invalid SendingType values.The function lacks an else clause and will return
Noneiftype_doesn't match any expected value. Past review comments indicate this was addressed, but the current code doesn't include the fix.Apply this diff to add proper error handling:
def get_client(type_: SendingType) -> mt.MailtrapClient: if type_ == SendingType.DEFAULT: return mt.MailtrapClient(token=API_TOKEN) elif type_ == SendingType.BULK: return mt.MailtrapClient(token=API_TOKEN, bulk=True) elif type_ == SendingType.SANDBOX: return mt.MailtrapClient( token=API_TOKEN, sandbox=True, inbox_id="<YOUR_INBOX_ID>", ) + else: + raise ValueError(f"Invalid sending type: {type_}")
🧹 Nitpick comments (1)
examples/sending/batch_advanced_sending.py (1)
9-12: Consider extracting SendingType to a shared module.This class is duplicated across all six example files. While acceptable for examples, consider extracting it to a shared constants or utils module to reduce duplication and improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(4 hunks)examples/sending/advanced_sending.py(1 hunks)examples/sending/batch_advanced_sending.py(1 hunks)examples/sending/batch_minimal_sending.py(1 hunks)examples/sending/batch_sending_with_template.py(1 hunks)examples/sending/minimal_sending.py(1 hunks)examples/sending/sending_with_template.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
examples/sending/batch_sending_with_template.py (3)
mailtrap/client.py (1)
MailtrapClient(31-170)mailtrap/models/mail/batch_mail.py (3)
BatchSendEmailParams(53-55)BatchMailFromTemplate(31-33)BatchEmailRequest(37-49)mailtrap/models/mail/address.py (1)
Address(9-11)
examples/sending/sending_with_template.py (3)
mailtrap/client.py (1)
MailtrapClient(31-170)mailtrap/models/mail/mail.py (2)
MailFromTemplate(33-35)BaseMail(13-21)mailtrap/models/mail/address.py (1)
Address(9-11)
examples/sending/minimal_sending.py (3)
mailtrap/models/mail/mail.py (3)
SendingMailResponse(39-41)BaseMail(13-21)mailtrap/client.py (2)
MailtrapClient(31-170)sending_api(97-99)mailtrap/models/mail/address.py (1)
Address(9-11)
examples/sending/advanced_sending.py (4)
mailtrap/client.py (2)
MailtrapClient(31-170)headers(137-144)mailtrap/models/mail/mail.py (2)
BaseMail(13-21)mailtrap/models/mail/address.py (1)
Address(9-11)mailtrap/models/mail/attachment.py (1)
Disposition(12-14)
examples/sending/batch_advanced_sending.py (4)
mailtrap/client.py (2)
MailtrapClient(31-170)headers(137-144)mailtrap/models/mail/batch_mail.py (3)
BatchSendEmailParams(53-55)BatchMail(23-27)BatchEmailRequest(37-49)mailtrap/models/mail/address.py (1)
Address(9-11)mailtrap/models/mail/attachment.py (1)
Disposition(12-14)
examples/sending/batch_minimal_sending.py (4)
mailtrap/models/mail/batch_mail.py (4)
BatchSendResponse(66-69)BatchSendEmailParams(53-55)BatchMail(23-27)BatchEmailRequest(37-49)mailtrap/client.py (2)
MailtrapClient(31-170)sending_api(97-99)mailtrap/models/mail/address.py (1)
Address(9-11)mailtrap/models/mail/mail.py (1)
BaseMail(13-21)
🪛 Ruff (0.14.1)
examples/sending/batch_sending_with_template.py
3-3: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/sending/sending_with_template.py
3-3: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/sending/minimal_sending.py
4-4: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/sending/advanced_sending.py
6-6: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/sending/batch_advanced_sending.py
6-6: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
examples/sending/batch_minimal_sending.py
4-4: Possible hardcoded password assigned to: "API_TOKEN"
(S105)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test python3.9 on macos-latest
- GitHub Check: Test python3.12 on windows-latest
- GitHub Check: Test python3.13 on windows-latest
- GitHub Check: Test python3.10 on windows-latest
- GitHub Check: Test python3.9 on windows-latest
- GitHub Check: Test python3.11 on windows-latest
🔇 Additional comments (12)
examples/sending/batch_advanced_sending.py (2)
31-89: LGTM!The batch mail configuration demonstrates advanced features effectively, including inline attachments, HTML content, custom headers, and multiple recipients as requested in previous reviews.
92-101: LGTM!The batch_send wrapper and main block are clear and demonstrate the usage effectively.
examples/sending/sending_with_template.py (1)
23-44: LGTM!The template-based sending example is clear and demonstrates the API correctly.
examples/sending/minimal_sending.py (1)
32-46: LGTM!Demonstrating both
client.send()andclient.sending_api.send()approaches provides valuable comparison for users.examples/sending/batch_sending_with_template.py (1)
23-56: LGTM!The batch template sending example demonstrates the functionality clearly with appropriate template variables.
examples/sending/batch_minimal_sending.py (1)
24-39: LGTM!The batch minimal sending example demonstrates both approaches effectively.
Also applies to: 42-46, 56-59
examples/sending/advanced_sending.py (1)
31-86: LGTM!The advanced sending example effectively demonstrates inline attachments, HTML content, custom headers, and variables.
README.md (5)
30-48: LGTM!The minimal usage section provides a clear starting point with appropriate placeholders for sensitive values.
50-95: Excellent environment-based configuration guidance.This section effectively demonstrates the recommended practice of using environment variables for configuration and provides clear instructions for switching between sandbox and production modes.
96-156: LGTM!The full-featured example comprehensively demonstrates advanced capabilities including attachments, HTML content, and custom headers with environment-based token retrieval.
177-216: Valuable addition: direct SendingApi usage.This new section clearly explains the difference between
client.send()andclient.sending_api.send()with concrete examples of their different return types. This addresses the past review comment about showing newer design patterns.
217-294: Comprehensive usage examples and functionality documentation.The categorized list of examples and supported functionality provides excellent navigation and clarity about SDK capabilities.
Motivation
In this PR I updated README.md file to start using the latest guideline
Changes
Summary by CodeRabbit
Documentation
New Features
Removals
Breaking Changes
Tests