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

Add new SQL Server provider package that uses Microsoft.Data.SqlClient #2063

Merged
merged 35 commits into from
Feb 14, 2023

Conversation

ErikEJ
Copy link
Contributor

@ErikEJ ErikEJ commented Nov 26, 2022

For your consideration 😄

fixes #823

WIP Based on the provider here which has more than 150.000 downloads.

Approach is conditional compiliation combined with as few required duplications as possible.

Dropped support for SQL Server 2000 (support removed in MDS)

I plan to update to M.D.S. 5.1 when it arrives in January 2023.

TBD

  • naming of provider, # symbol name, and replacement classes.
    We will go with Microsoft.EntityFramework.SqlServer for the package and assembly name. (Using EntityFramework.SqlServer would be problematic since that the assembly name of the current provider.)
  • approcah to testing?
    @ajcvickers to do manual testing
  • packaging (I do not really understand the current packaging process)
  • drop support for SQL 2005/2008? (my opinion is keep it to avoid code changes).
    We will not drop support or make other code changes

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Nov 26, 2022

See some CI warnnings:

"C:\Windows\system32\cmd.exe" /D /E:ON /V:OFF /S /C "CALL "D:\a_work_temp\a62e249e-3feb-4ea3-b751-69aa15afbde9.cmd""
_AdditionalBuildArgs : The term '_AdditionalBuildArgs' is not recognized as the name of a cmdlet, function, script
file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct
and try again.

@ajcvickers
Copy link
Member

@ErikEJ I owe you a case of beer!

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Nov 27, 2022

🤣🍺 - looking forward!

@ajcvickers
Copy link
Member

Thanks, @ErikEJ, this looks great! Will discuss with the team on naming and the other open issues you list.

@dougbu Any comments on the build errors or the packaging of the new package?

@bricelam Can you also take a look.

@dougbu
Copy link
Member

dougbu commented Nov 28, 2022

@dougbu Any comments on the build errors or the packaging of the new package?

  1. What build errors @ajcvickers❔ I suspect the last commit fixed things up.
  2. Package will be produced automatically once <IsPackable>True</IsPackable> is added to the new project. Note however that, because this is from a fork, the new package will not be in uploaded artifacts; will just see it created in the log.
  3. nit: I suggest using a longer define name than MDS. That acronym won't mean much to people looking at some of the changed files in isolation.

Side note: Please ping @dotnet/aspnet-build in general. I'm not always available.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Nov 28, 2022

What is a "define name" ?

@dougbu
Copy link
Member

dougbu commented Nov 28, 2022

What is a "define name" ?

Sorry, a C# constant. See the new project and its $(DefineConstants) setting as well as most of the C# files which use #if MDS.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Nov 28, 2022

@dougbu Got it. Yes, I assume that it will be considered as part of the "naming" discussion

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Nov 29, 2022

@dotnet/aspnet-build I see this warning in the build log - can it safely be ignored?

_AdditionalBuildArgs : The term '_AdditionalBuildArgs' is not recognized as the name of a cmdlet, function, script 
file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct 
and try again.
At line:1 char:131
+ ...  -ci -configuration Release -prepareMachine   $(_AdditionalBuildArgs)
+                                                     ~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : ObjectNotFound: (_AdditionalBuildArgs:String) [], CommandNotFoundException
    + FullyQualifiedErrorId : CommandNotFoundException

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Nov 29, 2022

Package created and signed 🎉

Successfully created package 'D:\a\_work\1\s\artifacts\packages\Release\Shipping\Microsoft.EntityFramework.SqlServer.6.5.0-ci.nupkg'.

@dougbu
Copy link
Member

dougbu commented Nov 29, 2022

@dotnet/aspnet-build I see this warning in the build log - can it safely be ignored?

_AdditionalBuildArgs : The term '_AdditionalBuildArgs' is not recognized as the name of a cmdlet, function, script 
file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct 
and try again.

@ErikEJ yes, this can safely be ignored in this PR. We should fix it, but the problem is completely unrelated to your changes.

@ajcvickers
Copy link
Member

I think this is ready to merge. @bricelam @ErikEJ Do you concur?

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 7, 2023

LGTM, especially after @bricelam suggestion to use a project reference!

I am ready to assist with supporting this!

Co-authored-by: Arthur Vickers <ajcvickers@hotmail.com>
@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 11, 2023

I am getting thirsty 🍺🤣

@ajcvickers
Copy link
Member

@ErikEJ I would hit merge, but it looks like it needs rebasing first, and I'm always worried I'll mess that up.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 11, 2023

Ah. Let me do that!

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 11, 2023

Done! @ajcvickers

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 14, 2023

Any more blockers @ajcvickers ??

@ajcvickers ajcvickers merged commit be77956 into dotnet:main Feb 14, 2023
@ajcvickers
Copy link
Member

Thanks for all your work on this, @ErikEJ. I will now talk to @dougbu about getting a preview build of 6.5 that we can put on NuGet.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 14, 2023

Fantastic! It was a pleasure to be allowed to contribute...

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Mar 14, 2023

Any official plans for a preview? @ajcvickers

@MartinInnovend
Copy link

How should I upgrade to this 'new' EF 6 ?

Just get the latest MS EntityFramework NuGet Package ?
I see the latest is 6.4.4. But I guess I need a new (preview) version 6.5 ?

We hope the copy dll issues will be solved then.
Still having that problem. Strange thing is that I made a workaround by letting MS Build copy those 2 dll's. But that doesnt always work. Sometimes though it works. The file copy is always done, and I can see those file copies, but for some reason the web application still gives the error that those 2 dll's are missing.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Mar 15, 2023

@MartinInnovend 6.5 preview1 has not been published to NuGet yet. In the meantime you can just use my package.

@ajcvickers
Copy link
Member

@ErikEJ We're trying to work through release mechanics.

@timomta
Copy link

timomta commented Apr 6, 2023

Hello! I have some customers waiting on this fix and I'm trying to understand the status of it from this thread. It looks like it is fixed but not released yet, is that correct?
When it is released, will it just be a matter of updating the nuget package?
Is there an ETA on release of the fix?

@ajcvickers
Copy link
Member

It's likely this will go GA in November.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Apr 7, 2023

@ajcvickers and a preview?

@ajcvickers
Copy link
Member

Yes, I'm hoping to get it included with the monthly previews starting soon.

@summitabrown
Copy link

@ajcvickers Any timeframe on a preview package?

@dishantbuch
Copy link

dishantbuch commented Jun 5, 2023

@ajcvickers Any timeframe on a preview package? Also can you confirm that it will go GA in November? because we are planning to include Micosoft.Data.SQLClient migration in our roadmap for Jan 2024 release.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Nov 18, 2023

For any watching this, any release plans have been postponed due to dependency issue in M.D.S. that should be resolved in M.D.S 5.1.4 planned for January 2024

@ksysiekj
Copy link

@ErikEJ any updates for now? can see that MDS was recently released https://www.nuget.org/packages/Microsoft.Data.SqlClient/5.1.4

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jan 13, 2024

@ksysiekj I did a PR, waiting for @ajcvickers to respond. Not having high hopes, tbh.

Are you not able to use my package?

@ksysiekj
Copy link

@ErikEJ I tried, but it seems that it doesn't work well with Z.EntityFramework.Classic version 7.2.*. I think you have to release ef 6.5 first, and then I have to wait for the changes in Z.EntityFramework.Classic, and then I can use it in my projects.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jan 14, 2024

@ksysiekj Z.EntityFramework.Classic is a seperate fork of EntityFramework, and not related to this change. If you want that seperate provider to use Microsoft.Data.SqlClient, none of the recent changes hare are going to help you. You must ask the maintainer of Z.EntityFramework.Classic to add a new provider for that, or move from Z.EntityFramework.Classic to ErikEJ.EntityFramework.SqlServer (and maybe eventually to Microsoft.EntityFramework.SqlServer )

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

Successfully merging this pull request may close these issues.

Consider supporting Microsoft.Data.SqlClient in EF6
10 participants