-
Notifications
You must be signed in to change notification settings - Fork 471
Updated ScriptJobHostOptions to implement IOptionsFormatter #11470
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
Conversation
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 adds serialization support to ScriptJobHostOptions by implementing the IOptionsFormatter interface, enabling the class to format its options as JSON. This allows the options to be logged or displayed consistently with other option types in the codebase.
- Implements
IOptionsFormatterinterface with a custom JSON converter - Adds comprehensive test coverage for the
Format()method - Serializes key properties:
FileWatchingEnabled,FileLoggingMode,FunctionTimeout, andTelemetryMode
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/WebJobs.Script/Config/ScriptJobHostOptions.cs | Implements IOptionsFormatter interface with a custom JsonConverter to selectively serialize specific properties |
| test/WebJobs.Script.Tests/Configuration/ScriptJobHostOptionsTests.cs | Adds unit tests verifying JSON serialization behavior including null handling and formatting |
test/WebJobs.Script.Tests/Configuration/ScriptJobHostOptionsTests.cs
Outdated
Show resolved
Hide resolved
support JSON serialization with a custom converter. Added a Format method for serializing options to JSON. Introduced unit tests to validate serialization logic.
d77986f to
68cab91
Compare
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public override void Write( |
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.
Why do we need custom JsonConverter?
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.
I've considered adding the JsonSourceGenerationOptions attribute to use source generation to create a SerializationContext as we do in other Options classes. However, since we didn't want to log all the properties but a few, almost all the properties would have ended with a JsonIgnore attribute and didn't like that approach.
Issue describing the changes in this PR
This PR implements
IOptionsFormatterforScriptJobHostOptions:{ "FileWatchingEnabled": true, "FileLoggingMode": "DebugOnly", "FunctionTimeout": "01:00:00", "TelemetryMode": "ApplicationInsights" }resolves #11460
Pull request checklist
IMPORTANT: Currently, changes must be backported to the
in-procbranch to be included in Core Tools and non-Flex deployments.in-procbranch is not requiredrelease_notes.md