fix (timezone/windows): Implemented support for the tzlocal#189
fix (timezone/windows): Implemented support for the tzlocal#189jafin wants to merge 2 commits intoMaciek-roboblog:mainfrom
tzlocal#189Conversation
… (version 5.0+) which properly converts Windows timezone IDs to IANA timezone names, with silent fallback to the original `tzutil` approach if `tzlocal` is not available.
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds optional tzlocal support and runtime detection for Windows timezones: module-level HAS_TZLOCAL and get_localzone_name stub; Windows detection tries tzlocal IANA name first, then falls back to tzutil. pyproject.toml lists tzlocal as a Windows-only dependency and tests for Windows behaviors were added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/tests/test_time_utils.py`:
- Around line 362-374: The test misses mocking os.environ.get("TZ") so it may
short-circuit and skip the Windows branch; update test_get_timezone_windows to
ensure TZ is unset by adding a mock for os.environ.get that returns None (e.g.,
add `@patch`("os.environ.get", return_value=None) or use patch.dict to clear TZ)
before calling SystemTimeDetector.get_timezone(), so the test always exercises
the Windows logic and uses mock_get_localzone_name -> "America/New_York".
🧹 Nitpick comments (1)
pyproject.toml (1)
48-49: Consider makingtzlocala Windows-only dependency.
tzlocalis only used in the Windows code path ofSystemTimeDetector.get_timezone(), and the import is already guarded withtry/except. Installing it unconditionally on Linux/macOS adds an unnecessary transitive dependency. Apply the same platform marker used fortzdata:Proposed fix
"tzdata; sys_platform == 'win32'", - "tzlocal>=5.0" + "tzlocal>=5.0; sys_platform == 'win32'"
|
@jafin I'm really keen to see better time zone support in Australia. I'm not as experienced in Python, so did you get a chance to see the Code Rabit automated review? #189 (review) |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@Maciek-roboblog - Would you mind reviewing this PR to help the software engineers who are in different time zones work with your great product? |
Windows Timezone Detection Fix - Implementation Summary
Problem
The Windows timezone detection in
SystemTimeDetector.get_timezone()is usingtzutilwhich returns Windows-specific timezone IDs (e.g., "Eastern Standard Time", "Pacific Standard Time") instead of IANA timezone names (e.g., "America/New_York", "America/Los_Angeles"). This caused failures for some locales and inconsistent behavior across platforms.Solution
Implemented support for the
tzlocallibrary (version 5.0+) which properly converts Windows timezone IDs to IANA timezone names, with silent fallback to the originaltzutilapproach iftzlocalis not available.Changes Made
1. Added tzlocal Import (src/claude_monitor/utils/time_utils.py)
2. Updated Windows Timezone Detection (src/claude_monitor/utils/time_utils.py)
3. Added tzlocal Dependency (pyproject.toml)
4. Updated Tests (src/tests/test_time_utils.py)
Added two tests to cover both the tzlocal and fallback scenarios:
test_get_timezone_windows: Tests tzlocal-based IANA timezone detectiontest_get_timezone_windows_fallback: Tests fallback to tzutil when tzlocal is unavailableResolves #188
Summary by CodeRabbit
Bug Fixes
Tests
Chores