FIX(api-client) Handle ReportPortal API timestamp format change#8
FIX(api-client) Handle ReportPortal API timestamp format change#8
Conversation
ReportPortal API changed from Unix milliseconds to ISO format strings. Added normalization layer in API client to support both formats for backward compatibility. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> fixes #7 Signed-off-by: Zdenek Kraus <zkraus@redhat.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 47 minutes and 22 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded timestamp normalization utilities to convert ISO‑8601 strings (including Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/reportportal/rp_api_client.py`:
- Around line 100-101: The parameter annotation for normalize_timestamps_in_dict
uses an implicit optional (List[str] = None); update the type hint to
Optional[List[str]] for the timestamp_fields parameter (i.e., def
normalize_timestamps_in_dict(self, data: Dict[str, Any], timestamp_fields:
Optional[List[str]] = None) -> Dict[str, Any]) so it conforms to PEP
484/RUF013—Optional is already imported, so just change the annotation and keep
the default None.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 368d02bb-7892-46ce-a672-a721d7afafd3
📒 Files selected for processing (2)
src/reportportal/rp_api_client.pytests/unit/test_rp_api_client.py
Signed-off-by: Zdenek Kraus <zkraus@redhat.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/reportportal/rp_api_client.py (1)
68-98: Consolidate ISO timestamp parsing to avoid code duplication and drift.The implementation duplicates ISO format conversion logic already present in
src/reportportal/junit_parser.py. This creates a second source of truth; the next time the API format changes, the two implementations may diverge. The codebase includes offset-less ISO timestamps (e.g."2024-01-01T12:00:00"without timezone), which rely ondatetime.fromisoformat()andtimestamp()to handle naive datetimes as local time. Consider extracting the ISO parsing into a shared utility and documenting the timezone assumption explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/reportportal/rp_api_client.py` around lines 68 - 98, The ISO parsing logic in normalize_timestamp duplicates logic in src/reportportal/junit_parser.py; extract the ISO-to-datetime logic into a single shared utility (e.g., parse_iso_datetime or parse_iso_timestamp) and have normalize_timestamp call that utility for string inputs instead of doing its own fromisoformat/replace('Z','+00:00') logic; update junit_parser to use the same utility, add a short docstring on the utility explaining the timezone assumption for offset-less ISO strings (that naive datetimes are treated as local time) and keep normalize_timestamp responsible only for converting the returned datetime to Unix milliseconds (int) or returning None on failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/reportportal/rp_api_client.py`:
- Around line 68-98: The ISO parsing logic in normalize_timestamp duplicates
logic in src/reportportal/junit_parser.py; extract the ISO-to-datetime logic
into a single shared utility (e.g., parse_iso_datetime or parse_iso_timestamp)
and have normalize_timestamp call that utility for string inputs instead of
doing its own fromisoformat/replace('Z','+00:00') logic; update junit_parser to
use the same utility, add a short docstring on the utility explaining the
timezone assumption for offset-less ISO strings (that naive datetimes are
treated as local time) and keep normalize_timestamp responsible only for
converting the returned datetime to Unix milliseconds (int) or returning None on
failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f15dd3d-df04-450d-b9c5-ef0f6162b759
📒 Files selected for processing (1)
src/reportportal/rp_api_client.py
…t change Signed-off-by: Zdenek Kraus <zkraus@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/reportportal/rp_api_client.py`:
- Around line 94-98: Replace the old-style percent formatting in the
logger.warning calls in rp_api_client.py with Loguru-style braces: update the
logger.warning in the except block that currently uses "Failed to parse
timestamp '%s': %s" to "Failed to parse timestamp '{}': {}" (passing timestamp
and e), and update the following logger.warning that currently uses "Timestamp
was unexpected type: %s" to "Timestamp was unexpected type: {}". These changes
should be made where timestamp parsing occurs (the except block and the
subsequent warning) so Loguru will interpolate the values correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0afc2813-8afd-48fb-bc50-56d775a09600
📒 Files selected for processing (2)
src/reportportal/rp_api_client.pytests/unit/test_rp_api_client.py
✅ Files skipped from review due to trivial changes (1)
- tests/unit/test_rp_api_client.py
…p format change Signed-off-by: Zdenek Kraus <zkraus@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/reportportal/rp_api_client.py (1)
90-93: Naïve ISO timestamps should be treated as UTC to prevent silent environmental drift.Line 92–93 converts ISO strings without explicit timezone handling. When an ISO timestamp lacks timezone information (no
Zor offset),datetime.fromisoformat()creates a naïve datetime object, and calling.timestamp()on it interprets it using the local machine timezone. This can silently skew millisecond values across different environments.The test suite indicates this is a known edge case (with comment "Without timezone, assumes local time") and the production API appears to always send properly formatted timestamps with timezone information (e.g.
+00:00). However, adding explicit UTC fallback for naïve inputs would improve robustness and prevent silent errors if the API contract changes.Suggested improvement
-from datetime import datetime +from datetime import datetime, timezone @@ if isinstance(timestamp, str): try: dt = datetime.fromisoformat(timestamp.replace('Z', '+00:00')) + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) return int(dt.timestamp() * 1000) except (ValueError, AttributeError) as e: logger.warning("Failed to parse timestamp '{}': {}", timestamp, e) return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/reportportal/rp_api_client.py` around lines 90 - 93, The conversion of ISO strings to milliseconds uses datetime.fromisoformat(timestamp.replace('Z', '+00:00')) and then .timestamp(), but if the parsed datetime is naïve (dt.tzinfo is None) it will be treated as local time and produce environment-dependent results; update the code after parsing to check dt.tzinfo and, if None, attach UTC (e.g., dt = dt.replace(tzinfo=timezone.utc) or otherwise convert to UTC) before calling int(dt.timestamp() * 1000) so naïve ISO inputs are interpreted as UTC; ensure you import datetime.timezone if needed and keep the existing handling of explicit 'Z' offsets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/reportportal/rp_api_client.py`:
- Line 461: The __init__ docstring of ReportPortalAPIClient is out of
date—remove references to the removed logger parameter and update the
constructor docstring to reflect the current signature
ReportPortalAPIClient(url, project, token); edit ReportPortalAPIClient.__init__
to delete any mention of a logger argument and clarify only the remaining
parameters (url, project, token) and their meanings so the docstring matches the
factory call that returns ReportPortalAPIClient(url, project, token).
---
Nitpick comments:
In `@src/reportportal/rp_api_client.py`:
- Around line 90-93: The conversion of ISO strings to milliseconds uses
datetime.fromisoformat(timestamp.replace('Z', '+00:00')) and then .timestamp(),
but if the parsed datetime is naïve (dt.tzinfo is None) it will be treated as
local time and produce environment-dependent results; update the code after
parsing to check dt.tzinfo and, if None, attach UTC (e.g., dt =
dt.replace(tzinfo=timezone.utc) or otherwise convert to UTC) before calling
int(dt.timestamp() * 1000) so naïve ISO inputs are interpreted as UTC; ensure
you import datetime.timezone if needed and keep the existing handling of
explicit 'Z' offsets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: decb0c22-caf7-4b0f-8e42-766a6b2ab249
📒 Files selected for processing (1)
src/reportportal/rp_api_client.py
…imestamp format change Signed-off-by: Zdenek Kraus <zkraus@redhat.com>
2 participants
ReportPortal API changed from Unix milliseconds to ISO format strings. Added normalization layer in API client to support both formats for backward compatibility.
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
fixes #7
Signed-off-by: Zdenek Kraus zkraus@redhat.com
Note
Piggibagged a removal of optional logged that was superseded by using loguru. Even thou it is not related to this fix, it would create too much overhead to split.
Summary by CodeRabbit
New Features
Tests