Add ability to bind to SBReceivedMessage #1313

JoshLove-msft · 2023-02-06T18:57:20Z

Issue describing the changes in this PR

resolves #1535

Pull request checklist

My changes do not require documentation changes
- 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

JoshLove-msft · 2023-02-06T18:57:49Z

Need to figure out why E2E tests are not working.

JoshLove-msft · 2023-02-06T19:11:25Z

/cc @jsquire

jsquire

The Service Bus client library integrations look good to me.

extensions/Worker.Extensions.ServiceBus/src/ServiceBusExtensionStartup.cs

extensions/Worker.Extensions.ServiceBus/src/ServiceBusReceivedMessageConverter.cs

extensions/Worker.Extensions.ServiceBus/src/Worker.Extensions.ServiceBus.csproj

…t-worker into sb-received-message

extensions/Worker.Extensions.ServiceBus/src/ServiceBusReceivedMessageConverter.cs

SeanFeldman · 2023-02-19T19:34:47Z

Cross linking to the issues this PR fixes #384

liliankasem · 2023-03-14T23:53:43Z

Hey @JoshLove-msft, I've finally gotten round to writing a guide for deferred binding support in worker extensions - it's an initial draft so feedback is welcome: https://dev.azure.com/msazure/One/_git/AAPT-Antares-Docs?path=/TeamDocs/FunctionTeamDocs/EngineeringHowTos/DeferredBinding/DotnetIsolatedExtensionGuide.md&version=GBmaster&_a=preview

A couple things to call out which will help progress this PR:

There's info on how to write unit tests for the converter
There's info on how to write E2E tests for the extension
There's guidance on managed identity support

JoshLove-msft · 2023-03-15T00:34:45Z

Hey @JoshLove-msft, I've finally gotten round to writing a guide for deferred binding support in worker extensions - it's an initial draft so feedback is welcome: https://dev.azure.com/msazure/One/_git/AAPT-Antares-Docs?path=/TeamDocs/FunctionTeamDocs/EngineeringHowTos/DeferredBinding/DotnetIsolatedExtensionGuide.md&version=GBmaster&_a=preview

A couple things to call out which will help progress this PR:

There's info on how to write unit tests for the converter

There's info on how to write E2E tests for the extension

There's guidance on managed identity support

Thanks! I will take a look. But just to clarify this PR is blocked on guidance around how to handle the named parameter bindings.

JoshLove-msft · 2023-03-15T00:44:05Z

Hey @JoshLove-msft, I've finally gotten round to writing a guide for deferred binding support in worker extensions - it's an initial draft so feedback is welcome: https://dev.azure.com/msazure/One/_git/AAPT-Antares-Docs?path=/TeamDocs/FunctionTeamDocs/EngineeringHowTos/DeferredBinding/DotnetIsolatedExtensionGuide.md&version=GBmaster&_a=preview
A couple things to call out which will help progress this PR:

There's info on how to write unit tests for the converter

There's info on how to write E2E tests for the extension

There's guidance on managed identity support

Thanks! I will take a look. But just to clarify this PR is blocked on guidance around how to handle the named parameter bindings.

Regarding the guide, one thing that I couldn't find is how to set up E2E testing when using something other than Azurite. The only enabled E2E tests that I found all depend on Azurite so there isn't any need to reference secrets or deploy resources. Do you have information on how to configure such tests?

liliankasem · 2023-03-15T16:18:24Z

Hey @JoshLove-msft, I've finally gotten round to writing a guide for deferred binding support in worker extensions - it's an initial draft so feedback is welcome: https://dev.azure.com/msazure/One/_git/AAPT-Antares-Docs?path=/TeamDocs/FunctionTeamDocs/EngineeringHowTos/DeferredBinding/DotnetIsolatedExtensionGuide.md&version=GBmaster&_a=preview
A couple things to call out which will help progress this PR:

There's info on how to write unit tests for the converter

There's info on how to write E2E tests for the extension

There's guidance on managed identity support

Thanks! I will take a look. But just to clarify this PR is blocked on guidance around how to handle the named parameter bindings.

Regarding the guide, one thing that I couldn't find is how to set up E2E testing when using something other than Azurite. The only enabled E2E tests that I found all depend on Azurite so there isn't any need to reference secrets or deploy resources. Do you have information on how to configure such tests?

I don't believe we have anything in place for you to use real resources (definitely an improvement we can make to the scripts), but for now you can manually edit the Constants file. That's were all the emulator connection strings live so I imagine you can swap those out locally for testing.

liliankasem · 2023-03-16T18:58:14Z

Hey @JoshLove-msft, I've finally gotten round to writing a guide for deferred binding support in worker extensions - it's an initial draft so feedback is welcome: https://dev.azure.com/msazure/One/_git/AAPT-Antares-Docs?path=/TeamDocs/FunctionTeamDocs/EngineeringHowTos/DeferredBinding/DotnetIsolatedExtensionGuide.md&version=GBmaster&_a=preview
A couple things to call out which will help progress this PR:

There's info on how to write unit tests for the converter

There's info on how to write E2E tests for the extension

There's guidance on managed identity support

Thanks! I will take a look. But just to clarify this PR is blocked on guidance around how to handle the named parameter bindings.

Ah yup, we're working on that! We have a design in place and are working on a prototype. I'll flag this PR as blocked until we have something in place to unblock you here

JoshLove-msft · 2023-04-10T20:08:04Z

Hi @liliankasem,
Just wanted to check in to see if there is any update on the work needed to unblock this PR? Thanks!

liliankasem · 2023-04-10T22:03:08Z

Hi @liliankasem, Just wanted to check in to see if there is any update on the work needed to unblock this PR? Thanks!

Hello, we have a prototype in place for how we'd like to handle this. Once we have that fully implemented and merged, we'll do a new SDK release and you should be unblocked. @surgupta-msft any updates on how the skip deferred binding work is going/ETA?

We'll also make sure to update the author guide on how to use the new approach. The gist of it is that now the converter can use an attribute to define the type it supports e.g.

[SupportsDeferredBinding]
[SupportedConverterTypes(typeof(ServiceBusReceivedMessage))]
internal class ServiceBusReceivedMessageConverter : IInputConverter {}

surgupta-msft · 2023-04-12T19:07:19Z

Yup, here is the PR - #1462 for bypass deferred binding work. Working on getting this in in this sprint. @JoshLove-msft feel free to test it, otherwise I will update you as soon as this is released.

…zure-functions-dotnet-worker into sb-received-message

test/E2ETests/E2EApps/E2EApp/Blob/BlobTestFunctions.cs

test/E2ETests/E2ETests/HttpEndToEndTests.cs

NuGet.Config

extensions/Worker.Extensions.ServiceBus/src/ServiceBusExtensionStartup.cs

extensions/Worker.Extensions.ServiceBus/src/ServiceBusReceivedMessageConverter.cs

extensions/Worker.Extensions.ServiceBus/src/Worker.Extensions.ServiceBus.csproj

samples/Extensions/ServiceBus/ServiceBusFunction.cs

samples/Extensions/Extensions.csproj

samples/WorkerBindingSamples/ServiceBus/ServiceBusReceivedMessageBindingSamples.cs

test/FunctionMetadataGeneratorTests/FunctionMetadataGeneratorTests.cs

samples/WorkerBindingSamples/ServiceBus/ServiceBusReceivedMessageBindingSamples.cs

…zure-functions-dotnet-worker into sb-received-message

liliankasem

Almost there! just needs unit and e2e tests, feel free to use these as guidance:

extensions/Worker.Extensions.ServiceBus/src/ServiceBusReceivedMessageConverter.cs

release_notes.md

samples/WorkerBindingSamples/WorkerBindingSamples.csproj

surgupta-msft · 2023-05-18T17:58:48Z

Almost there! just needs unit and e2e tests, feel free to use these as guidance:

Cosmos unit test

Cosmos E2E test

I had a brief chat with Josh about E2E tests, but looks like ServiceBus does not have Emulator like Storage and Cosmos. Not sure if there is any other way we can do this?

liliankasem · 2023-05-18T19:37:30Z

Almost there! just needs unit and e2e tests, feel free to use these as guidance:

Cosmos unit test

Cosmos E2E test

I had a brief chat with Josh about E2E tests, but looks like ServiceBus does not have Emulator like Storage and Cosmos. Not sure if there is any other way we can do this?

Hmm in that case, we can leave E2E tests to be outside of this PR - it will require changes in our testing pipeline. One way around this would be to have our pipeline spin up a service bus resource, run the tests, then tear it all down.

Don't want to block this PR on that so we should a) still make sure to include unit tests, b) create an issue for updating our pipelines to deploy and destroy resources for E2E testing (could also be an investigation story if we want to explore other options), and c) create an issue to add service bus E2E tests (once infrastructure of it is ready)

JoshLove-msft · 2023-05-18T19:39:28Z

Don't want to block this PR on that so we should a) still make sure to include unit tests, b) create an issue for updating our pipelines to deploy and destroy resources for E2E testing (could also be an investigation story if we want to explore other options), and c) create an issue to add service bus E2E tests (once infrastructure of it is ready)

BTW we have invested a lot of effort to having live test infrastructure in the azure-sdk-for-net repo. So that would be another option you can consider - migrate the extension to that repo.

liliankasem · 2023-05-18T20:26:00Z

Don't want to block this PR on that so we should a) still make sure to include unit tests, b) create an issue for updating our pipelines to deploy and destroy resources for E2E testing (could also be an investigation story if we want to explore other options), and c) create an issue to add service bus E2E tests (once infrastructure of it is ready)

BTW we have invested a lot of effort to having live test infrastructure in the azure-sdk-for-net repo. So that would be another option you can consider - migrate the extension to that repo.

Yeah we have actually been thinking about moving all of the extensions out of this repo and into azure-sdk-for-net and azure-webjobs-sdk-extensions - definitely worth considering before we spend too much time setting up our infrastructure

JoshLove-msft · 2023-05-18T21:20:33Z

Almost there! just needs unit and e2e tests, feel free to use these as guidance:

Cosmos unit test

Cosmos E2E test

Unit tests added!

JoshLove-msft · 2023-05-18T22:44:29Z

@liliankasem @surgupta-msft thank you for the review and your help implementing this! Feel free to merge it.

* Add E2E tests for blob SDK type bindings (#1360) * Add analyzer for SupportsDeferredBindingAttribute (#1367) * Update SupportsDeferredBinding diagnostic code & update docs (#1377) * Revert "Remove types, tests & logic related to the SDK-binding feature (#1374)" This reverts commit eac5b19. * Added unit tests for BlobStorageConverter (#1370) * Adding test project * Added tests * Initial changes * Updating metadata * Adding a test * Code refactoring * Cosmos DB converter for SDK-type support and samples (#1406) * Execution flow added * Update pipeline variables to include tags (#1427) * Update extension variables to not use build number with tags (#1429) * Execution flow * cleanup * Cleanup * Cleanup * Cleanup execution flow * Added test * Adding tests * code cleanup * Code cleanup * code refactor * Added logic to check type before sending to converter * Updated tests * Removing advertised converters from options * Adding options back * Changes to check type * updating type check logic * correction in metadata generation * code cleanup * Test cleanup * Added changes to support poco * Metadata generation changes * Changes on Invocation side * correcting type check and tests * Test for poco invocation support * Added tests for poco invocation flows * Metadata generator minor update * Grpc definition update * Added poco and poco collection check * cleanup metadata generation * Adding support for type collection * code cleanup * Fixing metadata gen * Test fix * Grpc definition fix * Removing BlobContainerClient advertisement * Readding containerclient * Updated metadata generator * Changes as per latest design * code cleanup * updatin tests input * code cleanup * Cleanup Metadata generation * correcting converters fallback logic * Removing converter advertisement on cosmos trigger * Fixing failing test * test check in * Undo testing changes in sample app * spacing fix * updated tests * non secret changes * Addressing PR feedback * e2e test work * adding comments * Removed json deserialization attribute * Rename to AllowConverterFallback * Addressing feedback * Metada generation cleanup * Test cleanup * Addressing feedback * Updated grpcFunctionDefinition for multiple inputConverterAttribute * Removing cosmos converter registration * Address PR feedback * Address PR feedback * Test fix * Minor * addressing comments * getting e2e working * Implement bypass deferred binding (#1462) Implement bypass deferred binding * Build issue fix * comments * Update test/E2ETests/E2EApps/E2EApp/Table/TableInputBindingFunctions.cs Co-authored-by: Lilian Kasem <likasem@microsoft.com> * Adding check for PR in yml file (#1546) * addressing some more comments * dependency * one more dependency * trying to compile lol * seeing if tables project compiles with dependency * removing local instance * see if this compiles * trying to remove test * nvm adding it back * adding keys to nuget.config * trying to get feed * trying again * more yml * fixing spacing * ci.yml * Add ability to bind to SBReceivedMessage (#1313) * another change * trying again * formatting changes * changes * updating worker.sdk package * fixing unit test * getting unit tests to pass * fixing test --------- Co-authored-by: Lilian Kasem <likasem@microsoft.com> Co-authored-by: Surgupta <surgupta@microsoft.com> Co-authored-by: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com>

* Add E2E tests for blob SDK type bindings (Azure#1360) * Add analyzer for SupportsDeferredBindingAttribute (Azure#1367) * Update SupportsDeferredBinding diagnostic code & update docs (Azure#1377) * Revert "Remove types, tests & logic related to the SDK-binding feature (Azure#1374)" This reverts commit eac5b19. * Added unit tests for BlobStorageConverter (Azure#1370) * Adding test project * Added tests * Initial changes * Updating metadata * Adding a test * Code refactoring * Cosmos DB converter for SDK-type support and samples (Azure#1406) * Execution flow added * Update pipeline variables to include tags (Azure#1427) * Update extension variables to not use build number with tags (Azure#1429) * Execution flow * cleanup * Cleanup * Cleanup * Cleanup execution flow * Added test * Adding tests * code cleanup * Code cleanup * code refactor * Added logic to check type before sending to converter * Updated tests * Removing advertised converters from options * Adding options back * Changes to check type * updating type check logic * correction in metadata generation * code cleanup * Test cleanup * Added changes to support poco * Metadata generation changes * Changes on Invocation side * correcting type check and tests * Test for poco invocation support * Added tests for poco invocation flows * Metadata generator minor update * Grpc definition update * Added poco and poco collection check * cleanup metadata generation * Adding support for type collection * code cleanup * Fixing metadata gen * Test fix * Grpc definition fix * Removing BlobContainerClient advertisement * Readding containerclient * Updated metadata generator * Changes as per latest design * code cleanup * updatin tests input * code cleanup * Cleanup Metadata generation * correcting converters fallback logic * Removing converter advertisement on cosmos trigger * Fixing failing test * test check in * Undo testing changes in sample app * spacing fix * updated tests * non secret changes * Addressing PR feedback * e2e test work * adding comments * Removed json deserialization attribute * Rename to AllowConverterFallback * Addressing feedback * Metada generation cleanup * Test cleanup * Addressing feedback * Updated grpcFunctionDefinition for multiple inputConverterAttribute * Removing cosmos converter registration * Address PR feedback * Address PR feedback * Test fix * Minor * addressing comments * getting e2e working * Implement bypass deferred binding (Azure#1462) Implement bypass deferred binding * Build issue fix * comments * Update test/E2ETests/E2EApps/E2EApp/Table/TableInputBindingFunctions.cs Co-authored-by: Lilian Kasem <likasem@microsoft.com> * Adding check for PR in yml file (Azure#1546) * addressing some more comments * dependency * one more dependency * trying to compile lol * seeing if tables project compiles with dependency * removing local instance * see if this compiles * trying to remove test * nvm adding it back * adding keys to nuget.config * trying to get feed * trying again * more yml * fixing spacing * ci.yml * Add ability to bind to SBReceivedMessage (Azure#1313) * another change * trying again * formatting changes * changes * updating worker.sdk package * fixing unit test * getting unit tests to pass * fixing test --------- Co-authored-by: Lilian Kasem <likasem@microsoft.com> Co-authored-by: Surgupta <surgupta@microsoft.com> Co-authored-by: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com>

Add ability to bind to SBReceivedMessage

2647f1f

jsquire approved these changes Feb 6, 2023

View reviewed changes

extensions/Worker.Extensions.ServiceBus/src/ServiceBusExtensionStartup.cs Outdated Show resolved Hide resolved

kshyju reviewed Feb 6, 2023

View reviewed changes

extensions/Worker.Extensions.ServiceBus/src/ServiceBusExtensionStartup.cs Outdated Show resolved Hide resolved

Add assembly attribute

f00735c

liliankasem reviewed Feb 7, 2023

View reviewed changes

liliankasem mentioned this pull request Feb 8, 2023

[Epic] Support SDK-type bindings for out-of-proc workers #1081

Closed

2 tasks

JoshLove-msft added 2 commits February 15, 2023 21:46

Merge branch 'main' of https://github.com/Azure/azure-functions-dotne…

948e4ae

…t-worker into sb-received-message

PR fb

5478413

JoshLove-msft commented Feb 16, 2023

View reviewed changes

extensions/Worker.Extensions.ServiceBus/src/ServiceBusReceivedMessageConverter.cs Outdated Show resolved Hide resolved

JoshLove-msft commented Feb 16, 2023

View reviewed changes

extensions/Worker.Extensions.ServiceBus/src/ServiceBusReceivedMessageConverter.cs Outdated Show resolved Hide resolved

SeanFeldman mentioned this pull request Feb 19, 2023

With a ServiceBusTrigger, how to go from string/byte[] to Message? #384

Closed

liliankasem changed the base branch from main to feature/sdk-type-binding February 28, 2023 21:09

liliankasem added the blocked Items that cannot move forward at the moment. label Mar 16, 2023

JoshLove-msft added 3 commits May 16, 2023 15:07

save

2a41aed

Merge branch 'feature/sdk-type-binding' of https://github.com/Azure/a…

557d6fd

…zure-functions-dotnet-worker into sb-received-message

Use new attributes

a552f69

JoshLove-msft commented May 17, 2023

View reviewed changes

test/E2ETests/E2EApps/E2EApp/Blob/BlobTestFunctions.cs Outdated Show resolved Hide resolved

JoshLove-msft commented May 17, 2023

View reviewed changes

test/E2ETests/E2ETests/HttpEndToEndTests.cs Outdated Show resolved Hide resolved

JoshLove-msft commented May 17, 2023

View reviewed changes

NuGet.Config Outdated Show resolved Hide resolved

surgupta-msft reviewed May 18, 2023

View reviewed changes

samples/WorkerBindingSamples/ServiceBus/ServiceBusReceivedMessageBindingSamples.cs Outdated Show resolved Hide resolved

JoshLove-msft added 2 commits May 17, 2023 19:45

PR fb

3385c32

Merge branch 'feature/sdk-type-binding' of https://github.com/Azure/a…

b0bc62d

…zure-functions-dotnet-worker into sb-received-message

liliankasem reviewed May 18, 2023

View reviewed changes

JoshLove-msft added 2 commits May 18, 2023 14:08

PR fb

54efd0e

fix

41f51ba

liliankasem approved these changes May 18, 2023

View reviewed changes

liliankasem removed the blocked Items that cannot move forward at the moment. label May 18, 2023

Remove unnecessary registration

7e57462

surgupta-msft approved these changes May 20, 2023

View reviewed changes

liliankasem merged commit 14f203e into Azure:feature/sdk-type-binding May 22, 2023
18 checks passed

surgupta-msft pushed a commit that referenced this pull request May 22, 2023

Add ability to bind to SBReceivedMessage (#1313)

d25b34d

liliankasem pushed a commit that referenced this pull request May 22, 2023

Add ability to bind to SBReceivedMessage (#1313)

0d49e19

JoshLove-msft mentioned this pull request Jun 9, 2023

Support binding to EventData #1609

Merged

7 tasks

liliankasem pushed a commit that referenced this pull request Jun 21, 2023

Add ability to bind to SBReceivedMessage (#1313)

c20ac61

liliankasem pushed a commit that referenced this pull request Jun 22, 2023

Add ability to bind to SBReceivedMessage (#1313)

9c04836

liliankasem pushed a commit that referenced this pull request Jul 26, 2023

Add ability to bind to SBReceivedMessage (#1313)

f8959ed

liliankasem pushed a commit that referenced this pull request Aug 1, 2023

Add ability to bind to SBReceivedMessage (#1313)

9043b7a

JoshLove-msft added a commit to JoshLove-msft/azure-functions-dotnet-worker that referenced this pull request Oct 4, 2023

Add ability to bind to SBReceivedMessage (Azure#1313)

e26d1e6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to bind to SBReceivedMessage #1313

Add ability to bind to SBReceivedMessage #1313

JoshLove-msft commented Feb 6, 2023 •

edited by liliankasem

JoshLove-msft commented Feb 6, 2023

JoshLove-msft commented Feb 6, 2023

jsquire left a comment

SeanFeldman commented Feb 19, 2023

liliankasem commented Mar 14, 2023

JoshLove-msft commented Mar 15, 2023

JoshLove-msft commented Mar 15, 2023

liliankasem commented Mar 15, 2023

liliankasem commented Mar 16, 2023

JoshLove-msft commented Apr 10, 2023

liliankasem commented Apr 10, 2023

surgupta-msft commented Apr 12, 2023

liliankasem left a comment

surgupta-msft commented May 18, 2023

liliankasem commented May 18, 2023

JoshLove-msft commented May 18, 2023

liliankasem commented May 18, 2023

JoshLove-msft commented May 18, 2023

JoshLove-msft commented May 18, 2023

Add ability to bind to SBReceivedMessage #1313

Add ability to bind to SBReceivedMessage #1313

Conversation

JoshLove-msft commented Feb 6, 2023 • edited by liliankasem

Issue describing the changes in this PR

Pull request checklist

Additional information

JoshLove-msft commented Feb 6, 2023

JoshLove-msft commented Feb 6, 2023

jsquire left a comment

Choose a reason for hiding this comment

SeanFeldman commented Feb 19, 2023

liliankasem commented Mar 14, 2023

JoshLove-msft commented Mar 15, 2023

JoshLove-msft commented Mar 15, 2023

liliankasem commented Mar 15, 2023

liliankasem commented Mar 16, 2023

JoshLove-msft commented Apr 10, 2023

liliankasem commented Apr 10, 2023

surgupta-msft commented Apr 12, 2023

liliankasem left a comment

Choose a reason for hiding this comment

surgupta-msft commented May 18, 2023

liliankasem commented May 18, 2023

JoshLove-msft commented May 18, 2023

liliankasem commented May 18, 2023

JoshLove-msft commented May 18, 2023

JoshLove-msft commented May 18, 2023

JoshLove-msft commented Feb 6, 2023 •

edited by liliankasem