Giter VIP home page Giter VIP logo

Comments (8)

Camios avatar Camios commented on June 24, 2024 1

Hi @liliankasem

Just to clarify, a new solution with this function is working just fine as expected?

Yes

I found the upgrade assistant failed in the many ways but it was its oversight of not adding one or both of these packages to the project which is causing the runtime exception about missing file Microsoft.Bcl.AsyncInterfaces:
- Microsoft.ApplicationInsights.WorkerService
- Microsoft.Azure.Functions.Worker.ApplicationInsights

Here's a longer list of things the upgrade assistant didn't do to my existing Azure Function App project (mix of problems it left behind and things it doesn't do compared to new project):

  • Didn't add package dependencies:
    • Microsoft.Azure.Functions.Worker.Extensions.CosmosDB 
    • Microsoft.ApplicationInsights.WorkerService
    • Microsoft.Azure.Functions.Worker.ApplicationInsights
  • Didn't run a package restore after changing from in-process worker model based dependencies (WebJobs) to isolated worker model dependencies.
  • Possibly didn't trigger an Azure core tools update?? Might be a once-off.
  • Didn't convert CosmosDB parameter-based attribute output bindings (IAsyncCollector) to method-based CosmosDBOutput attributes and doesn't convert the parameter to a return value.
  • Didn't rename named binding parameters like collectionName and ConnectionStringSetting.
  • Didn't convert System.Web.Http usage:
    • new QueryCollection should have been converted to QueryString.Create
    • HttpRequest no longer has a ReadAsStringAsync method, needs to be converted to create StreamReader from HttpRequest.Body
    • Was using <Something>MessageResult and they needed to be converted to BadRequestObjectResult, StatusCodeResult, ObjectResults, etc
  • Had Dependency injection in startup.cs with [assembly: FunctionsStartup(typeof(Startup))] but I think it should have converted to HostBuilder.ConfigureServices in the newly added prrogram.cs and removed startup.cs?
  • In csproj it added <ImplicitUsings>enabled</ImplicitUsings>, but pretty sure "enabled" should be "enable" (or "true").
  • It went from using Newtonsoft.Json serializers to System.Text.Json but didn't convert any of the POCO models that were using Newtonsoft-specific attributes. This annoyingly causes runtime exceptions. And there's no way to swap back to Newtonsoft and System.Text.Json has functional differences which might be breaking (like a lack of property-level JsonProperty DefaultValue handling)
  • New project created a mix of files in the Properties folder, but they weren't added during the upgrade to the existing project. Does it need them?:
    • launchSettings.json
    • serviceDependencies.json
    • serviceDependencies.local.json
    • serviceDependencies.local.json.user
  • The new project also added <FrameworkReference Include="Microsoft.AspNetCore.App" /> which wasn't in the existing project wasn't added to the upgraded project. Should the upgrade assistant be adding AspNetCore framework reference to upgraded Function Apps too?

The upgrade assistant should be doing "best effort" to convert and showing a warning if there's a problem.

from azure-functions-dotnet-worker.

Camios avatar Camios commented on June 24, 2024

I tried generating a new solution for minimal reproduction, but I don't get the same problem.
I copied the classes out of the problematic solution into the new solution and it doesn't have the same problem.

from azure-functions-dotnet-worker.

liliankasem avatar liliankasem commented on June 24, 2024

Just to clarify, a new solution with this function is working just fine as expected?

It seems like the migration tool struggled here, so maybe there are improvements to make to that. However without a repro, it will be difficult to investigate this. It could be an issue with the project file, and/or nuget references - are you on all the latest packages for the worker, worker sdk, and cosmos extension?

Re: samples - we have more scenarios documented here: https://github.com/Azure/azure-functions-dotnet-worker/blob/main/samples/Extensions/CosmosDB/CosmosInputBindingFunctions.cs

from azure-functions-dotnet-worker.

liliankasem avatar liliankasem commented on June 24, 2024

Thank you for the detailed write up! I'm going to consider the original issue resolved and repurpose this issue to track migration assistant enhancements - sound fair?

from azure-functions-dotnet-worker.

Camios avatar Camios commented on June 24, 2024

From my perspective:

  1. I'm not blocked by the runtime exception anymore and the focus turns more to addressing improvements to the upgrade assistant.
  2. I did have some outstanding questions (I bolded them in my previous command) and I was hoping you could answer promptly to help my migration.

Re: samples - we have more scenarios documented here: https://github.com/Azure/azure-functions-dotnet-worker/blob/main/samples/Extensions/CosmosDB/CosmosInputBindingFunctions.cs

Thanks for the link, I just found the Examples on learn.microsoft.com were insufficient and ideally expanded. And if it doesn't exist then linking to the github samples from learn.microsoft.com page would be helpful too.

from azure-functions-dotnet-worker.

liliankasem avatar liliankasem commented on June 24, 2024

Had Dependency injection in startup.cs with [assembly: FunctionsStartup(typeof(Startup))] but I think it should have converted to HostBuilder.ConfigureServices in the newly added prrogram.cs and removed startup.cs?

Correct, DI should be moved to Program.cs and Startup.cs should be removed.

It went from using Newtonsoft.Json serializers to System.Text.Json but didn't convert any of the POCO models that were using Newtonsoft-specific attributes. This annoyingly causes runtime exceptions. And there's #2131 and System.Text.Json has functional differences which might be breaking (like a lack of property-level JsonProperty DefaultValue handling)

You should be able to configure a Newtonsoft serializer (example). There is unfortunately a bug with this when using AspNetCore .ConfigureFunctionsWebApplication. But you should still be able to do this using .ConfigureFunctionsWorkerDefaults if you don't need AspNetCore. The Cosmos extension uses the worker config serializer (as shown in the example linked) as the serializer for the CosmosClientOptions.

New project created a mix of files in the Properties folder, but they weren't added during the upgrade to the existing project. Does it need them?

I think you only need launchSettings.json for VS, the rest can probably be removed.

The new project also added which wasn't in the existing project wasn't added to the upgraded project. Should the upgrade assistant be adding AspNetCore framework reference to upgraded Function Apps too?

<FrameworkReference Include="Microsoft.AspNetCore.App" /> is only required if you want to use the new HTTP extension Worker.Extensions.Http.AspNetCore (same for ConfigureFunctionsWebApplication), otherwise you don't need it.

from azure-functions-dotnet-worker.

Camios avatar Camios commented on June 24, 2024

It went from using Newtonsoft.Json serializers to System.Text.Json but didn't convert any of the POCO models that were using Newtonsoft-specific attributes. This annoyingly causes runtime exceptions. And there's #2131 and System.Text.Json has functional differences which might be breaking (like a lack of property-level JsonProperty DefaultValue handling)

You should be able to configure a Newtonsoft serializer (example). There is unfortunately a bug with this when using AspNetCore .ConfigureFunctionsWebApplication. But you should still be able to do this using .ConfigureFunctionsWorkerDefaults if you don't need AspNetCore. The Cosmos extension uses the worker config serializer (as shown in the example linked) as the serializer for the CosmosClientOptions.

When debugging the Cosmos input binding's JSON deserializing, I saw Microsoft.Azure.Functions.Worker.CosmosDBConverter has a hard-coded JsonSerializerOptions when deserializing which would prevent any settings override in Program.cs.

Is that CosmosDBConverter only used by Function Apps with the ASP.NET Core integrations (is that the bug you refer to)?
Is a different DB converter used when the Function app doesn't use ASP.NET Core integration?

from azure-functions-dotnet-worker.

liliankasem avatar liliankasem commented on June 24, 2024

CosmosDBConverter has a hard-coded JsonSerializerOptions when deserializing which would prevent any settings override in Program.cs.

Yes it does have a hardcoded JsonSerializerOptions; what I was trying to convey is that there is a difference between that serializer and the cosmos client serializer that can be configured. The one that is hardcoded in the Converter is used when binding to a POCO. And the other is used by the CosmoClient, this one can be configured via workerOptions.Serializer.

Wanting the latter one to be configurable is perfectly valid feedback, and likely was an oversight. Please open a new issue to help us triage and address that.

Is that CosmosDBConverter only used by Function Apps with the ASP.NET Core integrations (is that the bug you refer to)?
Is a different DB converter used when the Function app doesn't use ASP.NET Core integration?

No, this converter is used for all Cosmos input bindings.

from azure-functions-dotnet-worker.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.