Fix issue #53: Add full usage in all examples #56
Conversation
WalkthroughExample/demo scripts were expanded with full CRUD flows, added conditional operations to avoid empty-result errors, refactored some direct prints into variables, and tightened sending client selection by raising ValueError for unrecognized sending types. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Example script
participant get_client as get_client(type_)
rect #DFF0D8
Caller->>get_client: call with type_ (DEFAULT/BULK/SANDBOX)
get_client-->>Caller: return corresponding client
end
rect #F8D7DA
Caller->>get_client: call with type_ (unknown)
get_client--xCaller: raise ValueError("Invalid sending type: {type_}")
Note right of get_client: Explicit error path added\n(previously could fall through)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/testing/messages.py (1)
48-77: Fix incorrect type annotations formessage_idparameter and return type.Multiple functions have incorrect type annotations:
- Lines 48, 52, 56, 60, 64, 68, 72, 76:
message_idshould beint, notstr(as confirmed by the API method signatures inmailtrap/api/resources/messages.py)- Line 76:
get_mail_headersshould returndict[str, Any], notstrThese mismatches will cause type-checking errors and mislead developers using these examples.
Apply this diff to fix the type annotations:
-def get_spam_report(inbox_id: int, message_id: str) -> SpamReport: +def get_spam_report(inbox_id: int, message_id: int) -> SpamReport: return messages_api.get_spam_report(inbox_id=inbox_id, message_id=message_id) -def get_html_analysis(inbox_id: int, message_id: str) -> AnalysisReport: +def get_html_analysis(inbox_id: int, message_id: int) -> AnalysisReport: return messages_api.get_html_analysis(inbox_id=inbox_id, message_id=message_id) -def get_text_message(inbox_id: int, message_id: str) -> str: +def get_text_message(inbox_id: int, message_id: int) -> str: return messages_api.get_text_message(inbox_id=inbox_id, message_id=message_id) -def get_raw_message(inbox_id: int, message_id: str) -> str: +def get_raw_message(inbox_id: int, message_id: int) -> str: return messages_api.get_raw_message(inbox_id=inbox_id, message_id=message_id) -def get_html_source(inbox_id: int, message_id: str) -> str: +def get_html_source(inbox_id: int, message_id: int) -> str: return messages_api.get_html_source(inbox_id=inbox_id, message_id=message_id) -def get_html_message(inbox_id: int, message_id: str) -> str: +def get_html_message(inbox_id: int, message_id: int) -> str: return messages_api.get_html_message(inbox_id=inbox_id, message_id=message_id) -def get_message_as_eml(inbox_id: int, message_id: str) -> str: +def get_message_as_eml(inbox_id: int, message_id: int) -> str: return messages_api.get_message_as_eml(inbox_id=inbox_id, message_id=message_id) -def get_mail_headers(inbox_id: int, message_id: str) -> str: +def get_mail_headers(inbox_id: int, message_id: int) -> dict[str, Any]: return messages_api.get_mail_headers(inbox_id=inbox_id, message_id=message_id)Note: You'll need to add
Anyto the imports at the top:-from typing import Optional +from typing import Any, Optional
🧹 Nitpick comments (3)
examples/testing/attachments.py (1)
28-32: Good defensive check to prevent IndexError.The conditional guard correctly prevents a runtime error when the attachments list is empty.
Optionally, consider adding user feedback when no attachments are found to make the example more informative:
attachments = list_attachments(inbox_id=INBOX_ID, message_id=MESSAGE_ID) print(attachments) if attachments: attachment = get_attachment( inbox_id=INBOX_ID, message_id=MESSAGE_ID, attachment_id=attachments[0].id ) print(attachment) + else: + print("No attachments found.")examples/sending/batch_advanced_sending.py (1)
9-26: Consider extracting common client setup logic.The
SendingTypeclass andget_clientfunction are duplicated across all 6 sending example files. While acceptable for self-contained examples, extracting this into a shared utility (e.g.,examples/sending/common.py) would reduce duplication and make maintenance easier.examples/testing/messages.py (1)
83-120: Consider demonstrating thedelete_messagefunction for completeness.The workflow comprehensively demonstrates most message operations, but the
delete_messagefunction (defined at lines 29-30) is not used. Adding a delete operation at the end would provide a complete CRUD demonstration and align with the PR objective of showing "full usage."Consider adding this at the end of the workflow:
headers = get_mail_headers(inbox_id=INBOX_ID, message_id=msg_id) print(headers) + + deleted = delete_message(inbox_id=INBOX_ID, message_id=msg_id) + print(deleted)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
examples/contacts/contact_fields.py(1 hunks)examples/contacts/contact_lists.py(1 hunks)examples/contacts/contacts.py(2 hunks)examples/email_templates/templates.py(1 hunks)examples/general/account_accesses.py(1 hunks)examples/general/accounts.py(1 hunks)examples/general/billing.py(1 hunks)examples/general/permissions.py(1 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)examples/suppressions/suppressions.py(1 hunks)examples/testing/attachments.py(1 hunks)examples/testing/inboxes.py(1 hunks)examples/testing/messages.py(1 hunks)examples/testing/projects.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
examples/general/billing.py (1)
mailtrap/api/resources/billing.py (1)
get_current_billing_usage(9-12)
examples/testing/attachments.py (1)
mailtrap/api/testing.py (1)
attachments(31-32)
examples/testing/messages.py (1)
mailtrap/api/resources/messages.py (8)
get_spam_report(100-103)get_html_analysis(105-108)get_text_message(110-114)get_raw_message(116-120)get_html_source(122-127)get_html_message(129-133)get_message_as_eml(135-139)get_mail_headers(141-146)
examples/testing/inboxes.py (1)
mailtrap/api/testing.py (1)
inboxes(23-24)
examples/email_templates/templates.py (1)
mailtrap/api/templates.py (1)
templates(11-12)
examples/testing/projects.py (1)
mailtrap/api/testing.py (1)
projects(19-20)
examples/suppressions/suppressions.py (1)
mailtrap/api/suppressions.py (1)
suppressions(11-12)
examples/general/permissions.py (2)
mailtrap/models/permissions.py (1)
PermissionResourceParams(26-30)mailtrap/api/resources/permissions.py (1)
bulk_permissions_update(20-39)
🪛 Ruff (0.14.2)
examples/sending/advanced_sending.py
26-26: Avoid specifying long messages outside the exception class
(TRY003)
examples/sending/batch_sending_with_template.py
21-21: Avoid specifying long messages outside the exception class
(TRY003)
examples/sending/minimal_sending.py
22-22: Avoid specifying long messages outside the exception class
(TRY003)
examples/sending/batch_minimal_sending.py
22-22: Avoid specifying long messages outside the exception class
(TRY003)
examples/sending/sending_with_template.py
21-21: Avoid specifying long messages outside the exception class
(TRY003)
examples/sending/batch_advanced_sending.py
26-26: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (11)
examples/general/accounts.py (1)
15-16: LGTM - Good refactor for example clarity.Assigning the result to a variable before printing makes the example more explicit and easier to debug.
examples/general/billing.py (1)
16-17: Approve the code changes as-is.The billing API only provides a single operation (
get_current_billing_usage), so the example correctly demonstrates its complete, full usage. The intermediate variable assignment is a valid improvement that enhances code readability without introducing issues.examples/sending/batch_advanced_sending.py (1)
26-26: Good defensive programming: explicit error for invalid input.The addition of the explicit
ValueErrorimproves error handling by preventing the function from implicitly returningNonewhen an invalid sending type is provided.examples/sending/advanced_sending.py (1)
26-26: LGTM: explicit error handling for invalid input.The
ValueErrorappropriately guards against invalid sending types.examples/sending/batch_minimal_sending.py (1)
22-22: LGTM: consistent error handling.The explicit
ValueErrorprevents silent failures with invalid sending types.examples/sending/sending_with_template.py (1)
21-21: LGTM: proper error handling.The explicit
ValueErrorimproves the example by demonstrating proper input validation.examples/sending/batch_sending_with_template.py (1)
21-21: LGTM: explicit error handling.Consistent with the pattern applied across all sending examples.
examples/sending/minimal_sending.py (1)
22-22: LGTM: completes consistent error handling across all examples.The explicit
ValueErroraligns with the pattern applied consistently across all 6 sending example files.examples/suppressions/suppressions.py (1)
21-26: LGTM! Clean demonstration with proper bounds checking.The example demonstrates a safe pattern by checking if suppressions exist before attempting to delete, preventing potential IndexError.
examples/contacts/contacts.py (1)
54-77: LGTM! Excellent CRUD demonstration pattern.The example consistently operates on the created entity throughout the workflow, and the read-back verification step (lines 73-74) demonstrates good practice for confirming updates.
examples/general/account_accesses.py (1)
22-30: LGTM! Safe demonstration pattern with proper bounds checking.The example demonstrates defensive coding by checking if accesses exist before attempting to delete, preventing potential IndexError.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/contacts/contact_fields.py(1 hunks)examples/contacts/contact_lists.py(1 hunks)examples/email_templates/templates.py(2 hunks)examples/general/permissions.py(1 hunks)examples/testing/inboxes.py(1 hunks)examples/testing/projects.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/contacts/contact_fields.py
- examples/testing/inboxes.py
- examples/general/permissions.py
🧰 Additional context used
🧬 Code graph analysis (1)
examples/email_templates/templates.py (4)
mailtrap/models/templates.py (1)
EmailTemplate(40-49)mailtrap/api/resources/templates.py (2)
get_by_id(20-23)delete(43-46)mailtrap/models/common.py (1)
DeletedObject(23-24)mailtrap/api/templates.py (1)
templates(11-12)
⏰ 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.13 on windows-latest
- GitHub Check: Test python3.9 on windows-latest
- GitHub Check: Test python3.10 on windows-latest
- GitHub Check: Test python3.9 on macos-latest
- GitHub Check: Test python3.12 on windows-latest
- GitHub Check: Test python3.11 on windows-latest
🔇 Additional comments (4)
examples/testing/projects.py (1)
31-45: LGTM! Previous critical issues resolved.The CRUD flow now correctly operates on the created project throughout. The previous IndexError risk from accessing
projects[0]has been eliminated, and all operations (get, update, delete) consistently usecreated.id.examples/contacts/contact_lists.py (1)
34-51: LGTM! Previous critical issues resolved.The CRUD flow now correctly operates on the created contact list throughout. The previous IndexError risk and the issue of operating on
lists[0](potentially a different list) have been eliminated. All operations consistently usecreated.id.examples/email_templates/templates.py (2)
35-57: LGTM! Type corrections align with underlying API.The changes from
strtointfortemplate_idparameters correctly align with the underlying API implementation inmailtrap/api/resources/templates.py, which already expectsintvalues.
61-85: Previous critical issues resolved.The CRUD flow now correctly operates on the created template throughout, using
created.idconsistently. The previous IndexError risk from accessingtemplates[0]has been eliminated.
Ihor-Bilous
requested review from
IgorDobryn,
VladimirTaytor,
andrii-porokhnavets and
yanchuk
Motivation
In this PR I updated all examples with full usage
Changes
Summary by CodeRabbit
New Features
Bug Fixes
Improvements