azure-webjobs-sdk-extensions icon indicating copy to clipboard operation
azure-webjobs-sdk-extensions copied to clipboard

[.NET Core] - Migrate NotificationHubs extension project

Open fabiocav opened this issue 7 years ago • 26 comments

Issue to track migration of the WebJobs.Extension.NotificationHubs project to .NET Standard 2.0

  • [ ] Migrate project
  • [ ] Setup build / artifacts publishing

fabiocav avatar Jun 28 '17 19:06 fabiocav

I started this migration here https://github.com/Azure/azure-webjobs-sdk-extensions/tree/migrate_notification_hubs but hit a wall. They don't appear to have a .NET Standard package. Using the existing package results in a bunch of runtime load failures on various system assemblies.

mathewc avatar Sep 01 '17 23:09 mathewc

NotificationHubs team has started working on .Net Core 2.0 SDK for Notification Hubs. ETA: 09/29.

mathewc avatar Sep 05 '17 17:09 mathewc

Is there any update on this?

BorisWilhelms avatar Mar 26 '18 20:03 BorisWilhelms

The notification hubs SDK for .net core 2.0 is available, so this porting work is unblocked. I don't have an ETA yet (right now we're focused on making sure the experience with our "core" bindings in V2 is smooth).

paulbatum avatar Mar 26 '18 21:03 paulbatum

@paulbatum - Are we tracking this item for GA?

pragnagopa avatar Jul 10 '18 18:07 pragnagopa

As of now, only preview version of NH nuget package is availablue https://www.nuget.org/packages/Microsoft.Azure.NotificationHubs/2.0.0-preview1

pragnagopa avatar Aug 10 '18 16:08 pragnagopa

We are not tracking this for V2 GA as the NH package is still in preview and my understanding is that this won't change in the near term.

paulbatum avatar Aug 21 '18 21:08 paulbatum

https://www.nuget.org/packages/Microsoft.Azure.NotificationHubs/2.0.0 is released.

pragnagopa avatar Sep 14 '18 18:09 pragnagopa

They changed their minds! Moving back into the backlog.

paulbatum avatar Sep 15 '18 20:09 paulbatum

Looking forward to having this extension for V2. Anything the community can do to help?

jsmarcus avatar Sep 24 '18 15:09 jsmarcus

@jsmarcus Sorry for the delay, the answer is yes! I've made a very quick attempt at a port here: https://github.com/paulbatum/azure-webjobs-sdk-extensions/tree/notificationhubs

If you grab this branch, you should be able to generate a nuget by running dotnet pack .\src\WebJobs.Extensions.NotificationHubs\.

I have done ZERO testing with this. I just tried to get the code moved over and compiling. Any chance you're up for trying it out?

paulbatum avatar Oct 10 '18 23:10 paulbatum

@paulbatum I've tried it and am running into an issue. Where should I log it?

jsmarcus avatar Oct 11 '18 15:10 jsmarcus

Lets just discuss it here since you're just working with bits on my fork.

paulbatum avatar Oct 11 '18 22:10 paulbatum

Sorry for the delay. Got caught up with other projects. Here is the exception I'm getting when trying to run my function app from command line.

Message=Unable to get constructor of NotificationHubsExtensionConfigProvider using provided constructor selector when resolving singleton NotificationHubsExtensionConfigProvider: IExtensionConfigProvider {ServiceKey=DefaultKey(4), ReturnDefaultIfNotRegistered} #105 in wrapper IEnumerable<IExtensionConfigProvider> as parameter "registeredExtensions" #1 in singleton DefaultExtensionRegistryFactory: IExtensionRegistryFactory #52

Full details here: https://gist.github.com/paulbatum/f7170955a46971456da2e69926af9d9d

jsmarcus avatar Oct 19 '18 17:10 jsmarcus

hope you dont mind - I edited your reply to move the details into a gist since they were so large :)

I'm taking a look now..

paulbatum avatar Oct 19 '18 18:10 paulbatum

Yeah there was an issue with the constructor visibility. I pushed a fix.

paulbatum avatar Oct 19 '18 19:10 paulbatum

This extension is working for me on my dev site. It's running as a v2.0 function app with incoming events from Event Grid.

One problem I had (which not everyone will have and isn't specifically related to your changes) was that I didn't want to specify my Hub Name in the attribute or host.json file. It took some work to figure out the correct app setting path to use. I don't think it's been documented (I haven't found it yet).

I am happy to share my project if you would like to see anything about it.

jsmarcus avatar Oct 22 '18 16:10 jsmarcus

Thats great. I'm glad its at least in a semi-usable state now. Can you share with me how you setup your connection string and hub name? When I ported this code over I was doing a bit of guesswork myself. For example if you can share the name of the app setting you used to specify the hub, that would be useful.

paulbatum avatar Oct 22 '18 16:10 paulbatum

So I found that there are several ways to configure the settings. I am using deploy from zip with a csharp library so I wanted all my settings to be in AppSettings. But here are the options as far as I'm aware.

Configure via attribute This requires the hub name to be specified for every function and cannot be changed at runtime. Also, I've listed the default connection string setting name here but it can be left off or a different name can be specified. If the attribute ConnectionStringSetting is not null or empty, the value is looked up in ConnectionStrings, AppSettings, and Environment variables, in that order.

[NotificationHub(ConnectionStringSetting = "NotificationHubs", HubName = "<HubName>")] NotificationHubClient notificationHubClient

Configure via host.json & local.settings.json This allows the hub name to be specified in one place but it also cannot be changed at runtime for my specific project (csharp library/run from zip). The connection string part is the same as for the above attribute but I'll show the json for it so see my notes from above.

host.json

{
  "version": "2.0",
  "extensions": {
    "notificationHubs": {
      "hubName": "<HubName>"
    }
  }
}

local.settings.json note that both connection strings are not necessary

{
  "ConnectionStrings": {
    "NotificationHubs": "<ConnectionString>"
  }
  "IsEncrypted": false,
  "Values": {
    "NotificationHubs": "<ConnectionString>"
  }
}

Configure via AppSettings IOptions configuration loading This is the method I chose to use as it allows me to change both settings in AppSettings and they are in the same place. I'll show the local.settings.json but it ends up being AppSettings when hosted on Azure. This was also the hardest to figure out as it's not documented (or is very tough to find).

local.settings.json

{
  "IsEncrypted": false,
  "Values": {
    "AzureFunctionsJobHost:extensions:NotificationHubs:ConnectionString": "<ConnectionString>",
    "AzureFunctionsJobHost:extensions:NotificationHubs:HubName": "<HubName>"
  }
}

jsmarcus avatar Oct 22 '18 18:10 jsmarcus

Anything else I can do?

jsmarcus avatar Oct 31 '18 16:10 jsmarcus

The next step is more mundane. I ported over the code but none of the unit tests. The tests for the old version live here: https://github.com/Azure/azure-webjobs-sdk-extensions/tree/v2.x/test/WebJobs.Extensions.Tests/Extensions/NotificationHubs

Our new project structure involves a separate test project for each extension as you can see here: https://github.com/Azure/azure-webjobs-sdk-extensions/tree/dev/test

So the next step involves creating a test project for notification hubs, porting the relevant tests over, confirming they run successfully, and checking to see if there are any test gaps (there might be a couple around how connection strings are handled since this changed in webjobs v3).

Let me know if you make any progress on this! Otherwise I think we should be able to move this forward on our side sometime during December.

paulbatum avatar Oct 31 '18 17:10 paulbatum

I've started working on these but I have run into a problem with not having a valid test connection string and hub name. I'll create a PR to your fork and I'll keep adding to it as I go. Hopefully you'll have a few minutes here or there to make sure I'm on the right path.

jsmarcus avatar Nov 05 '18 17:11 jsmarcus

@paulbatum do you know if there is an estimate for when this will be available?

ChrisAllisonMalta avatar May 22 '19 12:05 ChrisAllisonMalta

@ChrisAllisonMalta sorry for the lack of updates, this was deprioritized towards the end of last year and hasn't been picked back up. So while there is still no NuGet available, you could try to unblock yourself by building the bits in my fork (I provided instructions above). If you give this a try, it would be useful to hear whether the extension fulfills your scenario - the more reports we get that this is in a usable state, the better we can justify finishing the porting work and start publishing the nuget package.

paulbatum avatar May 25 '19 00:05 paulbatum

Hi Paul, thanks for the note. I’ll give it a go, not sure my development skills are up to it but stranger things have happened!

In the meantime as far as serverless is concerned is the recommendation to use a v1 function in a separate project?

ChrisAllisonMalta avatar May 25 '19 09:05 ChrisAllisonMalta

At this point in time my recommendation would be to use V2 and to reference the notification hubs client SDK instead of using an output binding. Obviously this is not ideal - output bindings typically require much less code to use. But it gets you on our latest bits and makes for a much simpler upgrade path once this work is complete.

paulbatum avatar May 29 '19 01:05 paulbatum