Skip to content

fix(hermes_time): read config yaml as utf-8 - #17740

Closed
acefun29 wants to merge 1 commit into
NousResearch:mainfrom
acefun29:pr/hermes-time-utf8-config
Closed

fix(hermes_time): read config yaml as utf-8#17740
acefun29 wants to merge 1 commit into
NousResearch:mainfrom
acefun29:pr/hermes-time-utf8-config

Conversation

@acefun29

Copy link
Copy Markdown

What does this PR do?

Fixes timezone config loading when config.yaml is UTF-8 but the platform default text encoding is different.

Previously, hermes_time.py opened the user's config.yaml without an explicit encoding. On systems where the default encoding is not UTF-8, a config file containing non-ASCII UTF-8 text could fail to decode and silently fall back to the local timezone. This PR reads the config as UTF-8 explicitly, matching how Hermes config files are normally written.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • Updated hermes_time.py to read config.yaml with encoding="utf-8".
  • Added a regression test in tests/test_timezone.py that simulates a non-UTF-8 default encoding while reading a UTF-8 config file.

How to Test

python -m pytest tests/test_timezone.py -q -o addopts=

Result:

19 passed, 2 skipped

Checklist

  • My commit messages follow Conventional Commits
  • My PR contains only changes related to this fix
  • I've added tests for my changes
  • I've tested on Windows 11, Python 3.11.9

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have area/config Config system, migrations, profiles labels Apr 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #8455 — same UTF-8 encoding fix for hermes_time.py config read. Part of broader #15519 effort.

@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #8455 — same UTF-8 encoding fix for hermes_time.py config read.

@teknium1

Copy link
Copy Markdown
Contributor

Closing as already implemented on main by the broader UTF-8 file-open sweep.

Evidence from this automated hermes-sweeper review:

Thanks for the focused Windows repro and regression-test contribution.

@teknium1 teknium1 closed this Jun 10, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants