-
Notifications
You must be signed in to change notification settings - Fork 174
Fix DateTime parsing fails on non-US locale runners #2047
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
|
@copilot don't forget the release notes 😉 |
Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
|
@copilot please fix the release notes. |
Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
... Fixed in commit 5702191. Moved the Issue 2045 entry to an unreleased section above the v8.1 version header. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a locale-dependent DateTime parsing bug that caused workflow failures on non-US locale runners (e.g., Australian locale). The issue occurred when DateTime values serialized in one locale format couldn't be parsed in another locale.
Key changes:
- Serialize DateTime using ISO 8601 format (
ToString("o")) for locale-agnostic representation - Parse DateTime with InvariantCulture and AdjustToUniversal flags to ensure UTC timezone handling
- Add comprehensive test to validate locale-agnostic serialization/deserialization
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Actions/WorkflowInitialize/WorkflowInitialize.ps1 | Changed DateTime serialization to use ISO 8601 format (ToString("o")) instead of default culture format |
| Actions/WorkflowPostProcess/WorkflowPostProcess.ps1 | Updated DateTime parsing to handle both DateTime objects and strings, using InvariantCulture and UTC conversion |
| Tests/WorkflowPostProcess.Test.ps1 | Added test case validating locale-agnostic DateTime serialization and UTC preservation |
| RELEASENOTES.md | Documented the fix for issue #2045 in the unreleased section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot Can you please resolve the merge conflicts? |
…ELEASENOTES conflict Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
@copilot can you look at it again? There are still merge conflicts. |
…onflicts Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
Resolved the new merge conflicts in commit dbce971. The RELEASENOTES.md has been updated to include both Issue 2045 and the new "AL-Go repositories with large amounts of projects" issue in the unreleased section at the top. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Call action | ||
|
|
||
| It 'Test DateTime serialization is locale-agnostic' { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't you need to set the locale in order for this test to test something meaningful? It seems like the issue came up because WorkflowPostProcess and WorkflowInitilize ran on different machines with different locales
| # Call action | ||
|
|
||
| It 'Test DateTime serialization is locale-agnostic' { | ||
| # Simulate what WorkflowInitialize does - serialize a datetime in ISO 8601 format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole test kind of just reimplements the logic from WorkflowPostprocess. Shouldn't we create a function in WorkflowPostprocess that we can call from here so we're testing the logic that actually runs in AL-Go workflows.
WorkflowInitialize.ps1(serialization) andWorkflowPostProcess.ps1(deserialization)WorkflowInitialize.ps1: Use ISO 8601 format (ToString("o")) for locale-agnostic serializationWorkflowPostProcess.ps1: Convert datetime to UTC before calculating durationSummary
This PR fixes a bug where DateTime parsing fails on non-US locale runners (e.g., Australian locale) in WorkflowPostProcess.ps1.
Root Cause: The workflowStartTime was being serialized using the current culture's date format, but when deserialized on a machine with a different locale, the format didn't match, causing parsing failures.
Solution:
ToString("o")) which is locale-independent.ToUniversalTime()orDateTimeStyles.AdjustToUniversalOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.