-
Notifications
You must be signed in to change notification settings - Fork 3
Support sandbox and bulk modes #12
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: deprecate-dead-python-versions
Are you sure you want to change the base?
Support sandbox and bulk modes #12
Conversation
81be95d
to
326b5e3
Compare
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.
Looks good overall! Just suggested improving the _host logic to make it more flexible and future-proof in case the sandbox host changes or needs to be overridden. Also added an exception to clarify errors when inbox_id is missing in sandbox mode. Great work so far!
@property | ||
def _host(self) -> str: | ||
if self.api_host: | ||
return self.api_host | ||
if self.sandbox: | ||
return self.SANDBOX_HOST | ||
if self.bulk: | ||
return self.BULK_HOST | ||
return self.DEFAULT_HOST | ||
|
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.
We should find a way to make it more flexible for future changes—hopefully unnecessary—but what if the sandbox host changes? In that case, we wouldn’t be able to handle it.
@property | |
def _host(self) -> str: | |
if self.api_host: | |
return self.api_host | |
if self.sandbox: | |
return self.SANDBOX_HOST | |
if self.bulk: | |
return self.BULK_HOST | |
return self.DEFAULT_HOST | |
@property | |
def _host(self) -> str: | |
if self.api_host and self.sandbox: | |
return self.api_host | |
if self.api_host and self.bulk: | |
return self.api_host | |
if self.sandbox: | |
return self.SANDBOX_HOST | |
if self.bulk: | |
return self.BULK_HOST | |
if self.api_host: | |
return self.api_host | |
return self.DEFAULT_HOST | |
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.
It's doubtful that the host will be changed, but the client supports api_host
, which has the highest priority and always takes part of the API URL if it's present, regardless of other parameters.
Introduced `sandbox` mode, allowing configuration for sandbox environments via `inbox_id`. Implemented validation to ensure correct usage of `sandbox` and `inbox_id`, along with related tests. Updated URL generation logic and error handling for enhanced flexibility.
Introduce a new `bulk` mode in the Mailtrap client, setting a dedicated `BULK_HOST` for bulk operations. Updated validation logic to prevent conflicts between bulk and sandbox modes. Added corresponding test cases to ensure correct functionality and URL generation.
326b5e3
to
a138158
Compare
Motivation
bulk
andsanbox
modes. This PR introduces this functionalityChanges
sanbox
mode support in MailtrapClientinbox_id
parameter to be setbulk
mode support in MailtrapClientHow to test
bulk=True
)sandbox=True, inbox_id=123
)