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.

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

Allow configuring the update refresh window #812

Merged
merged 1 commit into from
Feb 17, 2024

Conversation

kdomanski
Copy link
Contributor

This enables customizing the time of day within which periodic refresh occurs.

Previously docs said it happens between 12am-4am which is misleading, because the code effectively allowed refresh until 4:59:59 AM. The default window is now '0-5', which is practically the same as before.

The comment about checking every 24h hours has been fixed as well. This hasn't been true since commit c4fa76f

@@ -126,6 +124,18 @@ def __init__(self, config: ConfigHelper) -> None:
# Auto Status Refresh
self.refresh_timer: Optional[FlexTimer] = None
if auto_refresh_enabled:
self.refresh_window = config.getintlist('refresh_window', [0, 5],
Copy link
Owner

Choose a reason for hiding this comment

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

This should be moved outside of the condition. This way a user can toggle auto refresh on or off without removing "refresh_window" in the configuration.

In addition it guarantees that the self.refresh_window attribute is set. While it shouldn't be reachable when auto refresh is disabled, we avoid potential future bugs if there is a refactor that makes it reachable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. I forgot about the warning when an unused option is set. Fixed it now.
I could not decide whether the validation belongs up there with config.getintlist or right above the if auto_refresh_enabled: section. Let me know if you want it moved again.

@Arksine
Copy link
Owner

Arksine commented Feb 15, 2024

Thanks. Overall it looks like a good addition, I just had one comment.

This enables customizing the time of day within which periodic refresh occurs.

Previously docs said it happens between 12am-4am which is misleading,
because the code effectively allowed refresh until 4:59:59 AM.
The default window is now '0-5', which is practically the same as before.

The comment about checking every 24h hours has been fixed as well.
This hasn't been true since commit c4fa76f

Signed-off-by: Kamil Domański <[email protected]>
@Arksine Arksine merged commit 04b1103 into Arksine:master Feb 17, 2024
1 check passed
@Arksine
Copy link
Owner

Arksine commented Feb 17, 2024

Thanks.

@kdomanski kdomanski deleted the enable_auto_refresh branch February 17, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants