Make it possible to set a custom user agent#61
Conversation
📝 WalkthroughWalkthroughThe MailtrapClient now supports runtime customization of the User-Agent header. A new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mailtrap/client.py`:
- Line 63: The assignment to self._user_agent on the line using the ternary
expression (user_agent if user_agent is not None else self.DEFAULT_USER_AGENT)
exceeds the 90-char limit; fix it by splitting the expression across multiple
lines or by assigning the chosen value to a short temporary variable before
setting self._user_agent so that the final assignment line is under 90
characters — update the code around the self._user_agent assignment (where
user_agent and DEFAULT_USER_AGENT are referenced) to use a wrapped conditional
or intermediate variable to satisfy the line-length rule.
- Around line 1-5: Reorder and format the imports in mailtrap/client.py to
satisfy isort/Black: ensure stdlib imports are grouped together and alphabetized
(place import importlib.metadata and import warnings together in alpha order)
and include from typing import Optional in the correct stdlib group, then run
isort and black (e.g., isort mailtrap/client.py && black mailtrap/client.py) to
apply the final formatting; focus on the imports referencing importlib.metadata,
warnings, and Optional so CI pre-commit hooks pass.
- Around line 40-43: The module-level DEFAULT_USER_AGENT currently calls
importlib.metadata.version('mailtrap') at import time which raises
PackageNotFoundError in a raw checkout; change this to handle that exception
(catch PackageNotFoundError from importlib.metadata.version) and fall back to a
safe default version string (e.g., "0+unknown" or "local") or compute the
version lazily when needed; update the DEFAULT_USER_AGENT construction in
mailtrap.client to use the try/except fallback (or a helper like a
get_user_agent() function) so importing mailtrap.client no longer fails when the
package metadata is missing.
mailtrap/client.py
Outdated
| import warnings | ||
|
|
||
| import importlib.metadata | ||
|
|
||
| from typing import Optional |
There was a problem hiding this comment.
Fix import ordering and Black formatting to unblock CI.
The pre-commit hooks fail because import importlib.metadata and import warnings are stdlib imports that isort expects to be grouped together (no blank line between them) and sorted alphabetically (importlib < warnings). Black also flags a reformat of the file. Running isort mailtrap/client.py && black mailtrap/client.py locally should resolve both failures.
♻️ Proposed import fix (isort ordering)
-import warnings
-
-import importlib.metadata
-
-from typing import Optional
+import importlib.metadata
+import warnings
+from typing import Optional📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import warnings | |
| import importlib.metadata | |
| from typing import Optional | |
| import importlib.metadata | |
| import warnings | |
| from typing import Optional |
🧰 Tools
🪛 GitHub Actions: Tests and checks
[error] 1-1: isort: Import statements were reordered and the file was modified by the hook
[error] 1-1: Black: reformatted mailtrap/client.py
[error] 1-1: Pre-commit: The operation was canceled due to failures in hooks
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mailtrap/client.py` around lines 1 - 5, Reorder and format the imports in
mailtrap/client.py to satisfy isort/Black: ensure stdlib imports are grouped
together and alphabetized (place import importlib.metadata and import warnings
together in alpha order) and include from typing import Optional in the correct
stdlib group, then run isort and black (e.g., isort mailtrap/client.py && black
mailtrap/client.py) to apply the final formatting; focus on the imports
referencing importlib.metadata, warnings, and Optional so CI pre-commit hooks
pass.
| DEFAULT_USER_AGENT = ( | ||
| f"mailtrap-python/{importlib.metadata.version('mailtrap')} " | ||
| "(https://github.com/railsware/mailtrap-python)" | ||
| ) |
There was a problem hiding this comment.
PackageNotFoundError at module import time if package is not installed.
importlib.metadata.version('mailtrap') is evaluated when the class body is executed (i.e., when mailtrap.client is first imported). In a raw git checkout without pip install -e ., this raises PackageNotFoundError, making the entire module unimportable. Consider a fallback:
🛡️ Proposed fallback
+try:
+ _mailtrap_version = importlib.metadata.version("mailtrap")
+except importlib.metadata.PackageNotFoundError:
+ _mailtrap_version = "unknown"
+
class MailtrapClient:
...
DEFAULT_USER_AGENT = (
- f"mailtrap-python/{importlib.metadata.version('mailtrap')} "
+ f"mailtrap-python/{_mailtrap_version} "
"(https://github.com/railsware/mailtrap-python)"
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mailtrap/client.py` around lines 40 - 43, The module-level DEFAULT_USER_AGENT
currently calls importlib.metadata.version('mailtrap') at import time which
raises PackageNotFoundError in a raw checkout; change this to handle that
exception (catch PackageNotFoundError from importlib.metadata.version) and fall
back to a safe default version string (e.g., "0+unknown" or "local") or compute
the version lazily when needed; update the DEFAULT_USER_AGENT construction in
mailtrap.client to use the try/except fallback (or a helper like a
get_user_agent() function) so importing mailtrap.client no longer fails when the
package metadata is missing.
mailtrap/client.py
Outdated
| self.sandbox = sandbox | ||
| self.account_id = account_id | ||
| self.inbox_id = inbox_id | ||
| self._user_agent = user_agent if user_agent is not None else self.DEFAULT_USER_AGENT |
There was a problem hiding this comment.
Fix E501: line 63 exceeds the 90-character limit (92 chars).
This is flagged by flake8 and is blocking CI.
🐛 Proposed fix
- self._user_agent = user_agent if user_agent is not None else self.DEFAULT_USER_AGENT
+ self._user_agent = (
+ user_agent if user_agent is not None else self.DEFAULT_USER_AGENT
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self._user_agent = user_agent if user_agent is not None else self.DEFAULT_USER_AGENT | |
| self._user_agent = ( | |
| user_agent if user_agent is not None else self.DEFAULT_USER_AGENT | |
| ) |
🧰 Tools
🪛 GitHub Actions: Tests and checks
[error] 63-63: flake8: E501 line too long (92 > 90 characters)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mailtrap/client.py` at line 63, The assignment to self._user_agent on the
line using the ternary expression (user_agent if user_agent is not None else
self.DEFAULT_USER_AGENT) exceeds the 90-char limit; fix it by splitting the
expression across multiple lines or by assigning the chosen value to a short
temporary variable before setting self._user_agent so that the final assignment
line is under 90 characters — update the code around the self._user_agent
assignment (where user_agent and DEFAULT_USER_AGENT are referenced) to use a
wrapped conditional or intermediate variable to satisfy the line-length rule.
| BULK_HOST = BULK_HOST | ||
| SANDBOX_HOST = SANDBOX_HOST | ||
| DEFAULT_USER_AGENT = ( | ||
| f"mailtrap-python/{importlib.metadata.version('mailtrap')} " |
There was a problem hiding this comment.
Do we need a version though? I didn't see it advertised in our other SDKs. We'll need to update analytics to parse the version out for groupping
leonid-shevtsov
force-pushed
the
set-user-agent
branch
from
8fe4303 to
9e6c564
Compare
2 participants
Motivation
Make it possible to set a custom user agent to identify specific clients
Changes
Summary by CodeRabbit
New Features