Adding a new worker option property to opt-in the behavior to get empty entries in trigger payload #1091

kshyju · 2022-09-30T17:00:42Z

This change is part of a fix for 8499 which has details about what problem this is solving.

Adding a boolean property to WorkerOptions, using which customer can opt-in to get the behavior where empty entries in the message payload are not skipped. We made the host level changes in this PR to support this behavior.

Sample usage.

var host = new HostBuilder()
    .ConfigureFunctionsWorkerDefaults(builder =>
    {
    }, options =>
    {
        options.IncludeEmptyEntriesInPayload = true;
    })
    .Build();

Pull request checklist

My changes do not require documentation changes - Docs will be automatically published from type.
- Otherwise: Documentation issue linked to PR
My changes should not be added to the release notes for the next release
- Otherwise: I've added my notes to release_notes.md
My changes do not need to be backported to a previous version
- Otherwise: Backport tracked by issue/PR #issue_or_pr
I have added all required tests (Unit tests, E2E tests)

Additional information

Additional PR information

kshyju · 2022-09-30T17:07:16Z

src/DotNetWorker.Grpc/GrpcWorker.cs

            response.Capabilities.Add("HandlesWorkerTerminateMessage", bool.TrueString);
            response.Capabilities.Add("HandlesInvocationCancelMessage", bool.TrueString);

+            if (workerOptions.IncludeEmptyEntriesInMessagePayload)


If the option is disabled, we just do not send the capability at all (instead of sending with False value. Host code is not checking the value of this entry. It just checks for the existence of this capability key).

https://github.com/Azure/azure-functions-host/blob/ec3c2d1049351870bbf611760520cf1e48075c8f/src/WebJobs.Script.Grpc/MessageExtensions/GrpcMessageConversionExtensions.cs#L328-L331

fabiocav

Do we have a documentation item tracking public docs updates?

fabiocav · 2022-09-30T18:31:51Z

src/DotNetWorker.Core/Hosting/WorkerOptions.cs

        public InputConverterCollection InputConverters { get; } = new InputConverterCollection();
+
+        /// <summary>
+        /// Gets or sets a value that determines if empty entries should be included in the function trigger message payload.


It would be good to have more details here to make the behavior controlled by the option clearer, perhaps also outline the current behavior?

@fabiocav I tried to elaborate the scenario a bit more in the latest iteration. Could you take a look? Feel free to suggest a better version if we can improve this further. Thank you!

liliankasem · 2022-09-30T19:56:07Z

release_notes.md

 -->

- Add new analyzer rules for the source-generated function metadata preview (#1048)
- Bump gRPC dependencies to latest version (#1085)


Are you removing these because they're not public facing changes?

madelinegordon · 2022-09-30T20:46:50Z

test/DotNetWorkerTests/GrpcWorkerTests.cs

+        [Theory]
+        [InlineData("IncludeEmptyEntriesInMessagePayload", true, "IncludeEmptyEntriesInMessagePayload", true, "True")]
+        [InlineData("IncludeEmptyEntriesInMessagePayload", false, "IncludeEmptyEntriesInMessagePayload", false)]
+        public void InitRequest_ReturnsExpectedCapabilitiesBasedOnWorkerOptions(


nit - could space out the name to make easier to read "InitRequest_ReturnsExpectedCapabilities_BasedOnWorkerOptions"

madelinegordon · 2022-09-30T20:47:37Z

test/DotNetWorkerTests/GrpcWorkerTests.cs

+            bool shouldCapabilityPresent,
+            string expectedCapabilityValue = null)
+        {
+            var workerOption = new WorkerOptions();


nit - var name "workerOptions" ?

kshyju · 2022-10-06T19:46:59Z

Do we have a documentation item tracking public docs updates?

This public docs page for this type will get updated from the code comments once we merge to main. Do you think we should have a separate docs page for this property ? Let's evaluate whether we need to add details to this page.

…ty entries in function trigger payload

kshyju · 2022-10-10T23:14:13Z

/check-enforcer evaluate

kshyju · 2022-10-10T23:23:02Z

/check-enforcer reset

…ty entries in trigger payload (#1091) * Adding a new worker option property to opt-in the behavior to get empty entries in function trigger payload * Addressing PR feedback. * renaming variable name based on PR feedback. * Rebasing on top of main. Resolved release notes conflict.

kshyju commented Sep 30, 2022

View reviewed changes

kshyju requested review from brettsam, fabiocav, jviau, kashimiz, liliankasem, madelinegordon, satvu, soninaren and surgupta-msft September 30, 2022 17:08

satvu approved these changes Sep 30, 2022

View reviewed changes

fabiocav approved these changes Sep 30, 2022

View reviewed changes

liliankasem reviewed Sep 30, 2022

View reviewed changes

liliankasem approved these changes Sep 30, 2022

View reviewed changes

madelinegordon approved these changes Sep 30, 2022

View reviewed changes

jviau approved these changes Sep 30, 2022

View reviewed changes

kshyju mentioned this pull request Oct 4, 2022

Always send IncludeEmptyEntriesInPayload capability #1096

Closed

kshyju force-pushed the shkr/gh-1097-empty-payload-worker-options branch from 80bf0e7 to 9339b23 Compare October 5, 2022 19:16

kshyju added 4 commits October 10, 2022 15:53

Adding a new worker option property to opt-in the behavior to get emp…

b46ca9e

…ty entries in function trigger payload

Addressing PR feedback.

5d47f2a

renaming variable name based on PR feedback.

add09d4

Rebasing on top of main. Resolved release notes conflict.

96b8e29

kshyju force-pushed the shkr/gh-1097-empty-payload-worker-options branch from 9339b23 to 96b8e29 Compare October 10, 2022 22:56

kshyju merged commit 07068cb into main Oct 10, 2022

kshyju deleted the shkr/gh-1097-empty-payload-worker-options branch October 10, 2022 23:24

This was referenced Oct 17, 2022

Send IncludeEmptyEntriesInMessagePayload capability to host to get empty entries in payload Azure/azure-functions-python-worker#1122

Open

Allow users to specify IncludeEmptyEntriesInMessagePayload capability Azure/azure-functions-nodejs-library#175

Open

wesselkranenborg mentioned this pull request Nov 18, 2022

Null values are ignored by introduced new app setting to control skipping empty entries from event payload when sending to OOP layer Azure/azure-functions-host#8924

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adding a new worker option property to opt-in the behavior to get empty entries in trigger payload #1091

Adding a new worker option property to opt-in the behavior to get empty entries in trigger payload #1091

Uh oh!

kshyju commented Sep 30, 2022 •

edited

Loading

Uh oh!

kshyju Sep 30, 2022 •

edited

Loading

Uh oh!

fabiocav left a comment

Uh oh!

fabiocav Sep 30, 2022

Uh oh!

kshyju Oct 5, 2022

Uh oh!

liliankasem Sep 30, 2022

Uh oh!

madelinegordon Sep 30, 2022

Uh oh!

madelinegordon Sep 30, 2022

Uh oh!

kshyju commented Oct 6, 2022

Uh oh!

kshyju commented Oct 10, 2022

Uh oh!

kshyju commented Oct 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Adding a new worker option property to opt-in the behavior to get empty entries in trigger payload #1091

Adding a new worker option property to opt-in the behavior to get empty entries in trigger payload #1091

Uh oh!

Conversation

kshyju commented Sep 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Additional information

Uh oh!

kshyju Sep 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabiocav left a comment

Choose a reason for hiding this comment

Uh oh!

fabiocav Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

kshyju Oct 5, 2022

Choose a reason for hiding this comment

Uh oh!

liliankasem Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

madelinegordon Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

madelinegordon Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

kshyju commented Oct 6, 2022

Uh oh!

kshyju commented Oct 10, 2022

Uh oh!

kshyju commented Oct 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kshyju commented Sep 30, 2022 •

edited

Loading

kshyju Sep 30, 2022 •

edited

Loading