Skip to content
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

Switch to Microsoft.Data.SqlClient #374

Open
daniellittledev opened this issue Feb 26, 2020 · 37 comments
Open

Switch to Microsoft.Data.SqlClient #374

daniellittledev opened this issue Feb 26, 2020 · 37 comments

Comments

@daniellittledev
Copy link

Description

As Microsoft.Data.SqlClient is due to supersede/replace System.Data.SqlClient FSharp.Data.SqlClient should consider switching to Microsoft.Data.SqlClient.

@smoothdeveloper
Copy link
Collaborator

@daniellittledev thanks for creating this issue.

There is a comment regarding this: #337 (comment)

My current idea is that until the new client library provides for feature that would make it useful to consumers of the TP which can't be achieved with existing client, the disruption for existing consumers is going to be problematic.

I don't have experience switching codebase to new client library, and looking for feedback on this.

@daniellittledev have you started using Microsoft.Data.SqlClient?

@daniellittledev
Copy link
Author

@smoothdeveloper my initial interest for this change is that Microsoft.Data.SqlClient appears to have better support for dotnetcore, in particular this was the issue that got me thinking about it #373. I'd love to start using it but you always have to wait for your dependencies to support it first.

It may be worth taking the same approach as Microsoft.Data.SqlClient and fork or branch off somehow and provide two packages, one for the old SqlClient and one for the new.

@smoothdeveloper
Copy link
Collaborator

@daniellittledev do you know if this can achieved while keeping the same codebase and using compiler defines rather than forking the library implementation?

@daniellittledev
Copy link
Author

@smoothdeveloper I would assume so, but you would probably need two separate proj files as the dependencies would differ. They could both reference the same code and sit side by side.

I'm unsure if there is an better/easier way to do it.

@smoothdeveloper
Copy link
Collaborator

@daniellittledev you can have conditionals in the same fsproj as well for the reference.

@daniellittledev
Copy link
Author

@smoothdeveloper ah great, of course!

@daniellittledev
Copy link
Author

@smoothdeveloper I don't know much about how to implement type providers or how much time I have at the moment but I'm open to helping out if I can.

@smoothdeveloper
Copy link
Collaborator

@daniellittledev I'd be happy to help with this investigation effort but can't commit time authoring it right now.

I can help you get acquainted with the specificities related to TP, but those won't have significant interference with giving a try at changing the reference / changing the code.

If you are willing to take on this experiment, I'll be able to review, and help meet the gap to eventually allow shipping two versions based on a branch with a full port.

If you are looking to use the library with new client, I think you/someone giving it a try is a singificant enabling step.

@daniellittledev
Copy link
Author

@smoothdeveloper ok, point me to anything you think would be helpful, I'll see what I can do.

@smoothdeveloper
Copy link
Collaborator

@daniellittledev you'll first need to add the references, in paket.dependencies DesignTime group and paket.references next to https://github.com/fsprojects/FSharp.Data.SqlClient/tree/master/src/SqlClient.DesignTime

also add the normal nuget package reference in https://github.com/fsprojects/FSharp.Data.SqlClient/blob/master/src/SqlClient/SqlClient.fsproj

after this, you can try replacing all the namespaces to point to new types, and get the solution to build, then the test projects green.

If this can be reached, we will ponder on how to keep dual target for the time being.

If you make a fork, you can open an experimental PR here to allow others to peek into the work.

@charlesroddie
Copy link

My current idea is that until the new client library provides for feature that would make it useful to consumers of the TP which can't be achieved with existing client, the disruption for existing consumers is going to be problematic.

UTF-8 support and the fact that there is a problem with System.Data.SqlClient (#373) are good enough reasons IMO.

@daniellittledev
Copy link
Author

I've been looking a bit more at this, so a bit of an update

There are at least two dependencies that fail in a netstandard type provider:

System.Configuration.ConfigurationManager

This package is a Reference Assembly in netstandard and is only supported in net4x. Therefore it can not be used in a netstandard version.

Microsoft.SqlServer.Types

This package also only supports the net4x but it also has a dependency on System.Data.SqlClient which means no support in netstandard. The geospatial types it contains are also superseded by the NetTopologySuite package which EF Core now uses for dotnetcore support.

I'm still trying to get a working version, but I'll comment again when I do.

@smoothdeveloper
Copy link
Collaborator

@daniellittledev thanks a lot for the update.

So the two mentionned dependencies and features related to it may be put under conditional compilation.

I'm also thinking integrating the work for supporting 'Microsoft.Data.SqlClient' will be supported by an extra assembly / nuget package.

We will figure a way to support both from same codebase / solution, I think it is the best as it doesn't interfere with current consumers relying on the library and want to time their switch.

Going another way would likely deter usage of this library due to lots of incurred changes/disruption if we planned to just replace.

@daniellittledev
Copy link
Author

@smoothdeveloper I agree minimising disruption is a good approach. Although I've decided to get it working first then see how we can merge the two.

I've also discovered Microsoft.Data.SqlClient depends on System.Configuration.ConfigurationManager as well so it can't be removed. Anyhow, I do have an idea of what to try next, there might be a way to force the inclusion of the implementation assemblies assuming there is one for netstandard.

p.s. There's no such thing as a netcore type provider is there (or is it just netcore and netfx)?

@smoothdeveloper
Copy link
Collaborator

I think it is only netframework and netstandard that will be the targets.

@daniellittledev
Copy link
Author

daniellittledev commented May 24, 2020

After searching around for way too long I haven't found any hope or lead for loading the non-reference assembly in netstandard. However, it looks like type providers support netcoreapp fsprojects/FSharp.TypeProviders.SDK#328 which could get around the reference assembly issue.

But I haven't had much luck getting that to work either. The first problem is failing to load the DesignTime assembly from bin/typeproviders in the Test project.

'C:\Projects\FSharp.Data.SqlClient\bin\netcoreapp2.0\FSharp.Data.SqlClient.dll' reported an error: 
Assembly attribute 'TypeProviderAssemblyAttribute' refers to a designer assembly 'FSharp.Data.SqlClient.DesignTime.dll' which cannot be loaded or doesn't exist. 
Could not load file or assembly 'file:///C:\Projects\FSharp.Data.SqlClient\bin\netcoreapp2.0\FSharp.Data.SqlClient.DesignTime.dll' or one of its dependencies. 
The system cannot find the file specified.	SqlClient.Tests

If I manually copy the assembly into bin/netcoreapp2.0 it failed to load System.Runtime.

'C:\Projects\FSharp.Data.SqlClient\bin\netcoreapp2.0\FSharp.Data.SqlClient.dll' reported an error: 
Assembly attribute 'TypeProviderAssemblyAttribute' refers to a designer assembly 'FSharp.Data.SqlClient.DesignTime.dll' which cannot be loaded or doesn't exist. 
Could not load file or assembly 'System.Runtime, Version=4.2.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. 
The system cannot find the file specified.	SqlClient.Tests

I'm also trying to figure out how the Type Provider is loaded, or more specifically how can you verify which framework the tooling is running in. And how do type providers know which assemblies/package versions to load? There doesn't appear to be any other assemblies in the bin directory.

@samhanes
Copy link
Contributor

samhanes commented Aug 21, 2020

FWIW there is an issue tracking the lack of netstandard support for Microsoft.SqlServer.Types, but it's filed under "Ideas for Future" so I don't think there will be movement on it in the short term: dotnet/SqlClient#30.

Another possibility might be https://github.com/dotMorten/Microsoft.SqlServer.Types, but that library does not fully implement spatial operations for SqlGeography or SqlGeometry so I'm not sure it's a viable option.

@samhanes
Copy link
Contributor

I'm also trying to figure out how the Type Provider is loaded, or more specifically how can you verify which framework the tooling is running in. And how do type providers know which assemblies/package versions to load? There doesn't appear to be any other assemblies in the bin directory.

@daniellittledev As I understand it this is determined by the compiler - if you want to make use of features available in the netfx version of the DTC, you will have to compile with MsBuild rather than dotnet build. All features work with netcore at runtime, though.

@daniellittledev
Copy link
Author

@samhanes I mentioned NetTopologySuite which EF uses for spatial types #374 (comment)

@jackfoxy
Copy link

@smoothdeveloper IMO this project needs to make the leap to full net5.0/netstandard2.0 compatibility.

People on legacy framework can still use older versions.

Switching to Microsoft.Data.SqlClient is a "must have" for us as we frequently use other Sql libraries, which have all switched.

I can't recall exactly what the issues are but trying to work with both Data.SqlClient libraries at the same time is not good.

We have been using this project for over 7 years. Unfortunately we are forced to migrate all our projects to Dapper.

@daniellittledev
Copy link
Author

I might try and pick this back up again, I was never able to get past the runtime errors after getting it to compile against Microsoft.Data.SqlClient but it's probably time for another go. Although if anyone else wants to give it a shot as well, please feel free. Given that the move to dotnetcore is now in full swing this is a major risk to this project.

#375

@daniellittledev
Copy link
Author

I'm trying to find information on if Type Providers, particularly the DesignTime project, can target netcoreapp instead of netstandard. Because the some assemblies load the reference assembly instead of the implementation assembly at runtime. Does anyone know what FSharp Type Provider versions support which TargetFrameworks?

@charlesroddie
Copy link

@cartermp can you help with @daniellittledev 's last question?

@daniellittledev
Copy link
Author

I've written up more about what I've tried and the issue I'm having with loading reference assemblies here #375 (comment)

@cr3wdayt5p
Copy link

Maybe 2022 is the year we can switch to Microsoft.Data.SqlClient?

It seems that some of the libraries that previously caused trouble have been updated:

Microsoft.Data.SqlClient: https://www.nuget.org/packages/Microsoft.Data.SqlClient/5.0.0#supportedframeworks-body-tab

  • 5.0.0: netstandard2.0, netstandard2.1

System.Configuration.ConfigurationManager: https://www.nuget.org/packages/System.Configuration.ConfigurationManager/6.0.1#supportedframeworks-body-tab

  • 6.0.1: netstandard2.0

Microsoft.SqlServer.Types: https://www.nuget.org/packages/Microsoft.SqlServer.Types/160.900.6-rc0#supportedframeworks-body-tab

  • 160.900.6-rc0: netstandard2.1

Microsoft.SqlServer.TransactSql.ScriptDom: https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/150.4897.1#supportedframeworks-body-tab

  • 150.4897.1: netstandard2.0

@smoothdeveloper
Copy link
Collaborator

The way I am currently seeing it is we would compile / ship two versions of the type provider, likely compiled out of the same codebase, with different #define and maybe some refactoring to minimize the places it has an impact on.

It remains to see if the current code, even without this #define type of structure, would work with the new ADO.NET implementation.

@jackfoxy, @cr3wdayt5p, @charlesroddie, I am by no mean avoiding making this project take the leap to "spanking new" dotnet: I will carefully review and merge submitted PR that inch in that direction.

People on legacy framework can still use older versions.

As far as I am concerned, the project needs to keep working on legacy framework, even in the future; I think this is true for a significant share of consumers of this library, this is not what is blocking anything, AFAIU.

I am interested to know if people are actually using Microsoft.Data.SqlClient and what it does that System.Data.SqlClient doesn't.

@cr3wdayt5p
Copy link

I am specifically interested in 'Microsoft.Data.SqlClient' as it supports Azure's Managed Identity feature directly in the connection string. Hence it would be possible to use Managed Identity (secretless apps) with 'FSharp.Data.SqlClient' without any other code changes.

It even has the potential to eliminate the need for different connection strings for design and compile time.

Example connection string – not supported by 'System.Data.SqlClient':

Server=demo.database.windows.net; Authentication=Active Directory Managed Identity; Encrypt=True; Database=testdb

@charlesroddie
Copy link

charlesroddie commented Oct 6, 2022

As far as I am concerned, the project needs to keep working on legacy framework, even in the future; I think this is true for a significant share of consumers of this library, this is not what is blocking anything, AFAIU.

netstandard2.0 deals with this issue. It supports legacy frameworks going back to 4.6.1, which was released 7 years ago and is EOL. There is something really silly about jumping through hoops to have net40 support and it strongly indicates that this project is going to be unmaintainable.

@smoothdeveloper
Copy link
Collaborator

@cr3wdayt5p interesting, thanks for the feedback, I see this as interesting factual motivator, are you interested to try out to "convert" this repository, in a prototype branch?

@charlesroddie, please refrain from characterizing things in negative fashion (about things being "really silly"), just out of your own bitterness and maybe oversight about underlying reasons, which you don't seem to put attention in a generous fashion here, hence likely to be ignored (unlike PR that don't break anything and make the project advance).

I don't see a reason not to drop .net 4.0 and target net 4.8 instead (if nobody opposes to make this the lowest framework for new releases), given VS 2022 doesn't support .net 4.0 anymore but:

  • I'd not take for granted that netstandard2.0 "deals with this? issue" in context of .net framework targeted projects and consumers of this library
  • there are probably more careful steps to do, than what you did in Clean and modernize charlesroddie/FSharp.Data.SqlClient#1 if we want to improve the status of this repository, without breaking stuff
  • your comment don't bring anything about showing how the provider codebase could, say switch (in a prototype), but ultimately, support both System.Data.SqlClient and Microsoft.Data.SqlClient

I think this has not much to do with this issue, so @charlesroddie, if you want to help with "really silly" things, you may open an issue, or make narrowly targetted PR, that changes .NET 4.0 to .NET 4.8 as the lowest supported framework.

Back to the issue:

The library in this repository cannot drop the support for System.Data.SqlClient, but we can definitely sketch things to support both System.Data.SqlClient and Microsoft.Data.SqlClient, the first step is to have a prototype supporting the later, and having some design discussions around supporting both.

I hope this helps. If there is enough interest, as people seems to put it in comments here, and people put efforts into it, I'm sure we'll see a prototype for supporting Microsoft.Data.SqlClient emerge.

@cr3wdayt5p
Copy link

@smoothdeveloper I am still pretty new to F# and .NET in general. So I am probably not the right person to take on such a project. But I will be happy to give feedback and help test the changes.

Suggestion: How about making a clean break as a new major version, i.e. let v3.0.0 be .NET 6 and switch to Microsoft.Data.SqlClient only? This way all existing projects could continue to use the v2-branch (supporting older .NET versions and using System.Data.SqlClient).

Wouldn't it be simplere to support two "clean" versions rather than one convoluted project?

It would also make it easier for developers new to F#/.NET (like myself) to contribute to the project, e.g. I only have experience with .NET 6.0 so I don't know what is needed to support the older versions (and I don't have any motivation to learn that as all my own projects will be .NET 6.0 or higher).

@smoothdeveloper
Copy link
Collaborator

@cr3wdayt5p I'm not clear what I'm looking for would make the project significantly more convoluted, as we are talking about a type provider.

To me, maintaining two branches for bug fixes and features is a sure way to spend more time doing so and have both forks bifurcate than if it can consolidate on same code base (which I assume is doable, and maybe just a mater of adding overloads and managing a bit of state, not even needing #if).

This can only be assessed out of a prototype and such prototype, if it would drop System.Data.SqlClient would achieve what you are after.

Also I am not asking contributors / maintainers to care specifically for .net framework, if a contribution has glitches with it, I'm willing to pay a look and do what is doable to address it.

@daniellittledev
Copy link
Author

daniellittledev commented Mar 6, 2023

As far as I am concerned, the project needs to keep working on legacy framework, even in the future;

I think it's time to drop support for NET4.x, we're jeopardising the future of this project for a legacy framework.

@cartermp 's comments on this subject are pretty clear, it's not worth supporting in future versions source:

I think your best bet is to stop trying to support net461. Just move only to .NET Standard 2.0 and simplify everything.
It will truly be in the best long-term interest of this type provider to just target netstandard2.0 and build only with the .NET SDK compiler.
TP SDK was fragile/fraught with complexity. It is now entirely .NET Standard 2.0-focused.

I'm caught by this issue once again, and if I can't get this working on dotnetcore 6.0 I don't think I'll be able to use this project going forward.

I might have time for a final attempt at spiking Microsoft.Data.SqlClient.

@charlesroddie
Copy link

@smoothdeveloper earlier in the thread said it was OK to drop net framework < 4.8. Which means that going to netstandard2.0-only would be fine as netstandard2.0 supports framework 4.8 and even a few earlier versions. Can you clarify your position @smoothdeveloper so that @daniellittledev doesn't waste time?

@daniellittledev
Copy link
Author

daniellittledev commented Mar 9, 2023

Some notes on my recent spike:

While Microsoft.Data.SqlClient is a modern package with full support for .NET Core and the new .NET it has a few limitations with regards to it's usage in a type provider. The issues mostly stem from the fact that dotnet cannot load packages, it can only load assemblies. The assemblies that need to be loaded depend on the tooling runtime, because the TP runs in the tooling context.

Host tools include:

  • FsAutoComplete.exe .NET Framework or .NET Core
  • fsc.exe running on .NET Core 2.0 as either 64-bit or 32-bit
  • fsi.exe running on .NET Framework as 32-bit
  • fsiAnyCpu.exe running on .NET Framework as either 64-bit or 32-bit
  • fsc.exe running on .NET Framework as either 64-bit or 32-bit
  • devenv.exe running on .NET Framework 32-bit
  • The host process used by JetBrains Rider

See source F# Tooling RFC FST-1003 - Loading Type Provider Design-Time Components into F# Tooling

Then there are native platform specific components for win and unix that nuget packages can specify. So depending on the OS the correct runtime assemblies also need to use used.

The first issue I ran into is that the ref assembly is loaded for Microsoft.Data.SqlClient when building. This package has win and unix versions which you need to use to be able to actually connect to a database. This can be resolved with TypeProviderForNamespaces.ResolveAssembly and some custom code to find the correct assembly. I was able to get this working but I ran into a second issue.

The package Microsoft.Data.SqlClient depends on Microsoft.Data.SqlClient.SNI so that package needs a runtime assembly to be loaded as well.

Currently I'm trying to figure out how to load the dll for this package, which doesn't work because it's a native assembly and it does not trigger AppDomain.CurrentDomain.add_AssemblyResolve. Additionally there doesn't seem to be a way to dynamically load native assemblies in netstandard like with NativeLibrary. (See edit below for update)

To test it I'm building SqlClient.Tests on the command line with dotnet build. But I'm also getting a different error when I try to build the project in Visual Studio:

Root Error:
Could not load file or assembly 'netstandard, Version=2.1.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' or one of its dependencies.

Full Error:
The type provider 'C:\Projects\FSharp.Data.SqlClient\bin\netstandard2.1\FSharp.Data.SqlClient.dll' reported an error: The type provider designer assembly 'FSharp.Data.SqlClient.DesignTime.dll' could not be loaded from folder 'C:\Projects\FSharp.Data.SqlClient\bin\netstandard2.1'.
The exception reported was: System.IO.FileNotFoundException - Could not load file or assembly 'netstandard, Version=2.1.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' or one of its dependencies. The system cannot find the file specified.

The visual studio error has me stumped, I haven't seen anyone else run into issues loading a netstandard2.1 TP. Any help with this issue would be appreciated. netstandard2.1 is not supported in VS

This is all taking a huge amount of time, and I'm stuck at the moment. So if anyone else wants to try things as well my code is here #426 I have some leads for all the issues now, but I'm running out of time to look at this, so progress will be slower.

I wonder if I could avoid these issues by using a merged TP without a separate TPDTC.

Maybe @dsyme or @cartermp can provide some insight here?

Edits:

Native Assemblies
I might be able to solve the native assemblies here with:

Although there are four runtimes win-x64, win-x86, win-arm and win-64 but the example only shows two x86 and x64.
A quick manual test did not solve the issue, maybe the paths are wrong?

Visual Studio Error
Regarding VS error, is netstandard2.1 supported?

I also incorrectly had the design time dll in the bin\netstandard2.1 folder instead of only in bin\typeproviders\fsharp41\netstandard2.1. When the dlls are removed I get this error instead.

Error	FS3031	The type provider 'C:\Projects\FSharp.Data.SqlClient\bin\netstandard2.1\FSharp.Data.SqlClient.dll' reported an error:
Assembly attribute 'TypeProviderAssemblyAttribute' refers to a designer assembly 'FSharp.Data.SqlClient.DesignTime.dll' which cannot be loaded from path 'C:\Projects\FSharp.Data.SqlClient\bin\netstandard2.1\FSharp.Data.SqlClient.DesignTime.dll'.
The exception reported was: System.IO.FileNotFoundException -
Could not load file or assembly 'file:///C:\Projects\FSharp.Data.SqlClient\bin\netstandard2.1\FSharp.Data.SqlClient.DesignTime.dll' or one of its dependencies.
The system cannot find the file specified.	SqlClient.Tests (netcoreapp3.1)	C:\Projects\FSharp.Data.SqlClient\tests\SqlClient.Tests\Assert.fs

After some more digging I think VS doesn't support netstandard2.1 but dotnet build does, which explains the error:
The function toolingCompatibleVersions shows exactly what is supported.

More Notes

@daniellittledev
Copy link
Author

I am currently blocked due to being unable to load a native dll for Microsoft.Data.SqlClient.SNI.runtime on windows. See my full comment at dotnet/fsharp#3736 (comment)

@cr3wdayt5p
Copy link

@daniellittledev Maybe it is possible to avoid loading the native SNI by switching to managed SNI on Windows?

System.AppContext.SetSwitch("Switch.Microsoft.Data.SqlClient.UseManagedNetworkingOnWindows", true)

Sources:
https://learn.microsoft.com/en-us/sql/connect/ado-net/appcontext-switches?view=sql-server-ver16#enabling-managed-networking-on-windows
https://github.com/dotnet/SqlClient/blob/v2.0.0-preview3/release-notes/2.0/2.0.0-preview3.md#enabling-managed-networking-on-windows
dotnet/SqlClient#292 (comment)
https://www.compositional-it.com/news-blog/creating-f-scripts-for-sql-data-access-on-windows-net-core-3/

@daniellittledev
Copy link
Author

@cr3wdayt5p thanks, that does look promising!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants