BREAKING CHANGE: ScheduledTask: Allow better handling of multiple time zones #440
Conversation
|
@PlagueHO if you have time, please review this one. 🙂 |
|
@Borgquite seems the tests fail - guessing you are working on it? |
|
@johlju Yes - getting there, I'll post when done :) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #440 +/- ##
===================================
Coverage 87% 87%
===================================
Files 21 21
Lines 2081 2082 +1
===================================
+ Hits 1819 1820 +1
Misses 262 262
|
|
Oh cool. Let me get onto this tomorrow night |
|
@PlagueHO Thanks - let me know how it goes :) |
There was a problem hiding this comment.
Just the one query on the known issue.
Reviewed 4 of 9 files at r1, 1 of 5 files at r2, 4 of 4 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Borgquite and @PlagueHO)
source/DSCResources/DSC_ScheduledTask/README.md line 15 at r4 (raw file):
When creating a scheduled task with a StartTime, you should always specify both a date and a time, with the SortableDateTimePattern format (e.g. 1980-01-01T00:00:00). Not providing a date may result in 'flip flopping' if the remote server enters daylight savings time. The date and time specified will be set based on the time zone that has been configured on the device. If you want to synchronize a scheduled task across timezones, use the SynchronizeAcrossTimeZone parameter, and specify the timezone offset that is needed (e.g. 1980-01-01T00:00:00-08:00).
Should this potentially be enforced to ensure the user does not run into this (fixed in another issue maybe)?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dan-hughes and @PlagueHO)
source/DSCResources/DSC_ScheduledTask/README.md line 15 at r4 (raw file):
Previously, dan-hughes (Daniel Hughes) wrote…
Should this potentially be enforced to ensure the user does not run into this via another issue maybe?
An interesting thought. The difference between a Scheduled Task with SynchronizeAcrossTimeZone enabled, and one without, under the hood appears to be whether it is stored in UTC with the 'Z' offset, or without an offset at all.
I could see a situation where someone might not want to configure the offset (which, as of this change, should result in a task created using the system's configured timezone) but still wants SATZ enabled - it would work. It would be unusual, but perhaps someone might then want to export the scheduled task to XML and import it to another server, and therefore not defining an offset and SATZ could be desirable, even if very unusual.
In general I feel that we should only prevent something happening when it will cause an error - not just when it's perhaps 'not recommended'. The current behavior allows what is possible via the user interface (configuring a scheduled task and presuming the local timezone, even with SATZ enabled). I'd probably prefer to leave as-is (advice, not enforced) just in case there are edge cases like the one above where not defining the timezone but still enabling SATZ could be useful.
Makes sense. If someone does want to export and they've configured using DSC, it's not really a typical use case. @PlagueHO, looks good. |
Pull Request (PR) description
Various fixes for StartTime handling in ScheduledTask:
This Pull Request (PR) fixes the following issues
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
and comment-based help.
This change is