Skip to content
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.

Sign up for GitHub

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

Change FileWatcher to stop printing watchfiles debug log #340

Closed
wants to merge 1 commit into from
Closed

Change FileWatcher to stop printing watchfiles debug log #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/frequenz/channels/file_watcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ def __init__(
watch_filter=self._filter_events,
force_polling=force_polling,
poll_delay_ms=int(polling_interval.total_seconds() * 1_000),
# The underlying Rust code performs periodic checks to see if it should stop watching.
# These checks raise TimeoutError, which by default would log misleading debug messages.
# Setting yield_on_timeout=True makes it yield an empty set instead of printing debug
# logs. We handle these empty sets in the `ready()` method by continuing to wait.
yield_on_timeout=True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know what the impact of this would be on windows systems?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

samuelcolvin/watchfiles#110

Copy link
Contributor Author

@ela-kotulska-frequenz ela-kotulska-frequenz Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I think that if we change rust_timeout=sys.maxsize - 1, back to None.
Then it should work on Windows and won't print debug log. However I didn't test it.

This is because the timeout mechanism in awatch looks like this:

            if raw_changes == 'timeout':
                if yield_on_timeout:
                    yield set()
                else:
                    logger.debug('rust notify timeout, continuing')

So instead of printing debug log, it can yield set() and we can ignore it.
Only then debug logs won't print if it awatch was killed or not.

)
self._awatch_stopped_exc: Exception | None = None
self._changes: set[FileChange] = set()
Expand Down Expand Up @@ -204,11 +209,13 @@ async def ready(self) -> bool:
if self._awatch_stopped_exc is not None:
return False

try:
self._changes = await anext(self._awatch)
except StopAsyncIteration as err:
self._awatch_stopped_exc = err
return False
# awatch will yield an empty set if rust notify timeout is reached.
while len(self._changes) == 0:
try:
self._changes = await anext(self._awatch)
except StopAsyncIteration as err:
self._awatch_stopped_exc = err
return False

return True

Expand Down
1 change: 1 addition & 0 deletions tests/test_file_watcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ async def test_file_watcher_filter_events(
watch_filter=filter_events,
force_polling=True,
poll_delay_ms=1_000,
yield_on_timeout=True,
)
]
for event_type in EventType:
Expand Down