-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Port over DbProviderFactories class #15732
Comments
@FransBouma, you are correct that DataTable is not the only design issue we need to solve here to make DbProviderFactory and DbProviderFactories to work. The registration mechanism for Full Framework was through config files. For NET Core, we will have to come up with a new registration mechanism, but we don't have concrete design. This will be addressed post v1.0. |
Do realize that if this is introduced post v1.0, the v1.0 runtime will be a bit useless for ORMs: without a providerfactory, it's much harder to write db agnostic code that doesn't reference ADO.NET specific providers. .NET 1.x has learned a hard lesson: taking dependencies on ADO.NET providers in an ORM is a versioning nightmare. So not having any providerfactory support whatsoever (not even in the app config file or whatever it's called in corefx) is a showstopper. |
Just going to +1 this and say it should be all code based configuration. That's one thing I hated about those APIs in the past. |
I have just added the DbProviderFactory as a constructor parameter now and it can be passed in when you reference your particular database driver. Eg for sql server. SqlClientFactory.Instance It's a suitable workaround for now. I also agree with @davidfowl about a new code mechanism. However now that I think about it. You would have to register that in the app anyway so might as well just do what ive done. |
@davidfowl and @schotime, yes, the current thinking is to provide code based configuration. |
My micro orm Symbiotic is at %97+ to .NET Core compatibility. Just need DbProviderFactory and some config junk. |
Here our proposal for DbProviderFactories for corefx by using Code based Configuration As a background in .NET Framework As you probably know in CoreFx, there will be No GAC and global configuration anymore. @FransBouma, @eschneider999, @schotime , @davidfowl or others, Please let us know whether |
CC @giancarloa |
I understand the lack of GAC / machine.config makes it all a bit problematic and therefore would fall back onto the mechanism of the app's config file configuration for DbProviderFactory (similar to what is possible in .NET full today), and if configuration in CoreFX is done through code, not config files then this is the wisest solution. At first glance I see no way to obtain a factory when I have just an invariant name? As you register the factory under a name with There's another problem, which is perhaps less of your concern, and that's factory overwriting is not supported with your solution. ADO.NET profilers (like Glimpse, EFProf and also ORM Profiler) tend to overwrite the static table in .NET full with their own factories which intercept any call to the factory instance and create wrapping elements instead to intercept calls to ADO.NET elements. Your proposed solution doesn't offer that flexibility anymore. |
@FransBouma you can access your registered factory the same ways as the old ways in .NET Frameworks.
In CoreFx, you will need to use DbProviderFactoriesConfiguration to register your provider.
But in .NET Frameworks, you don't need to do anything, it will read from the machine config as it is. |
The reason why we are not allowing to overwrite is to make the behavior less uncertain. But an applications may be able to provide their own safe way to overwriting their provider config. |
Re: retrieving the factory: ah yes, it was unclear a bit to me whether that was changed or not, this morning I realized it was indeed the same. Sounds good! Re: overwriting: ok. It's not a big deal I think. The applications now also have to call a line in the profiler to make it overwrite the factories in the datatable, and instead of that the developer can simply register a different factory (i.e. the one from the profiler instead of the regular one). It's a bit more explicit but at least it is possible to do. Regarding dependencies: I think the main change is: in regular .NET, an application doesn't have to reference an ADO.NET provider assembly directly, and in CoreFX this will be different: The application has to reference the ADO.NET provider assembly directly as the registration of the factory (which is in the ADO.NET provider assembly) has to be registered. I think this unavoidable as there's no central GAC/machine.config so it is what it is. So IMHO it all looks OK to me: porting code over to CoreFX will now be easier as the mechanisms are the same. Thanks for this! |
Here's a question for you @FransBouma, how are you using this API? Since there's no config system and the application is responsible for wiring everything up anyways, why not just provide an API that takes a factory directly. It's up to user code to wire it up |
What do you mean with this question? As there's hardly any API to use: you call the static class' method to obtain the factory for a known name, you get the factory, cache it (as it's a static instance), and use the instance to create ADO.NET objects for using with a database. At least that's how I (and everyone else) use it on full .NET. On CoreFX I haven't used it yet, as it's impossible to create proper software with the pre-RC2 tooling.
No, that's way too simplistic. The application wires things up for the application, but it has not and should not know internals about the used ORM / Data access layer, other than perhaps which ADO.NET provider to use, but not how to set things up internally with that. Besides, it's not always known to the application when the ORM is used, or which ORM, or which database up front, only at runtime. It also makes things overly complicated as current code can now easily be ported to CoreFX because there's no direct line between application logic and the query engine of an ORM: it obtains the factory through the system. Your proposal requires code to be refactored because the factory has to be supplied directly to the query engine of the ORM by the application logic. In all honesty I then wonder what the real point of your question is: is this API being cut? |
@FransBouma sorry to put high expectation for this proposal initially. At this moment this proposal is currently under reviewed by internal teams which raised couple concern. And other concern is about "the original reason why we don't bring this DbProviderFactories in CoreFx". So at this moment, we need to gather more information from the community for the usage and suggest alternative way to use DbProviderFactory directly. We still need to do more analysis and threat modeling for this proposal. I will keep you up to date if there is any progress on this and please give us any input and suggestions Thanks |
Yeah, but that's a problem you have regardless: if you create a system to obtain the factory through methodology ABC in PCLs, it won't match what's done in normal .NET. If ADO.NET using code can't utilize DbProviderFactory on CoreFX, porting costs will become big. The last thing I want is my SQL engines having a dependency on the ADO.NET provider they indirectly are using. That sucked big time during .NET 1.x as every time the ADO.NET provider versioned, the ORM had to version as well, plus supporting multiple ADO.NET providers through one SQL engine (e.g. Oracle with ODP.NET with both a managed and a CLI wrapping ADO.NET provider, with at least 3 versions shipped across 2 CLR versions) is then out of the question and it's trivial to do today. I really don't care how the DbProviderFactory is registered, as long as ADO.NET using code (e.g. the SQL engines which create connections, DbCommand instances and the like) can obtain the DbProviderFactory through a central class by supplying a known name. If the application has to register the factory with the ORM, things get limited and you put the burden of how things are registered onto us ORM writers and our users instead of solving it yourselves properly. Anyway, I'm so tired debating trivial ADO.NET stuff with Microsoft time and time again. Do what you have to do, cut it, don't cut it, I don't really give a shit anymore. |
The main issue is: the ORM is perhaps used 3 layers deep. The application at start point doesn't have the notion of that ORM today. If the application has to do the registration of the factory with the ORM (which is now abstracted away!), this means that it has to go through these 3, 4 layers to reach the ORM to register the factory, a coupling that doesn't exist today with the DbProviderFactory system. Not only that, but the ORM has to be redesigned to make it possible to specify the factory through the outside, something that has to be changed with the current designs which simply obtain the factory from the system. This is all major pain and misery and additional burden for code to port to CoreFX. If it's your goal to make porting existing frameworks to CoreFX a miserable experience, you have your work cut out for you. Seriously, make it as easy as possible to port code, that's all we ask. It benefits us, but also you, as the last thing MS needs is lack of people willing to port code to CoreFX. Cheers. |
Maybe you should talk to the Entity Framework team, don't they use the dbproviderfactory also? -----Original Message----- The main issue is: the ORM is perhaps used 3 layers deep. The application at start point doesn't have the notion of that ORM today. If the application has to do the registration of the factory with the ORM (which is now abstracted away!), this means that it has to go through these 3, 4 layers to reach the ORM to register the factory, a coupling that doesn't exist today with the DbProviderFactory system. Not only that, but the ORM has to be redesigned to make it possible to specify the factory through the outside, something that has to be changed with the current designs which simply obtain the factory from the system. This is all major pain and misery and additional burden for code to port to CoreFX. If it's your goal to make porting existing frameworks to CoreFX a miserable experience, you have your work cut out for you. Seriously, make it as easy as possible to port code, that's all we ask. It benefits us, but also you, as the last thing MS needs is lack of people willing to port code to CoreFX. Cheers. You are receiving this because you were mentioned. |
Thanks @FransBouma for sharing your pain. No, we don't cut this API yet. @eschneider999 Yes, we already talked to EF Team and they have their own container / configuration mechanism.
so they don't use DbProviderFactories (Plurals) but they still use DbProviderFactory (Singular) for abstraction layer. DbProviderFactory (singular) is available in CoreFx since the beginning. |
Lol, there's the solution and they didn't even know it.... -----Original Message----- Thanks @FransBouma for sharing your pain. No, we don't cut this API. @eschneider999 Yes, we already talked to EF Team and they have their own configuration. so they don't use DbProviderFactories (Plurals) but they still use DbProviderFactory (Singular) for abstraction layer. DbProviderFactory (singular) is available in CoreFx since the beginning. You are receiving this because you were mentioned. |
As @kkurni said, we believe we've a design that satisfies the request. Here are the goals as I understood it by @FransBouma:
Specifically, that's the API shape for .NET Core as I discussed it with @kkurni: namespace System.Data.Common
{
public static class DbProviderFactories
{
// V1 -- Supported by .NET Framework 4.6.2 (the upcoming version)
public static DbProviderFactory GetFactory(string providerInvariantName);
public static DbProviderFactory GetFactory(DbConnection connection);
// V2 -- Supported by .NET Core 1.0 and by .NET Framework vNext (version after 4.6.2)
public static void RegisterFactory(string providerInvariantName, DbProviderFactory factory);
public static IEnumerable<string> GetFactoryProviderNames();
}
} Is that accurate? |
Yes. Application startup on CoreFX registers factories to use by application. Somewhere deep in the application's call chain, code can now ask the system for a factory using a well known name. In general these names are known up front. So e.g. the Oracle engine uses name X, and whatever is registered under X, be it the managed provider v12c, or the unmanaged v9i, it works. So for CoreFX all that's required is extra machinery to register a factory which is done on .net Full through machine.config / app.config, which then used by the application's startup code. Everything else between that and the low level dungeon of the data-access code is the same: DbProviderFactories.GetFactory is used to obtain a factory, providing a name X. Application architecture
In the situation above, A..F are components/layers which offer some form of abstraction in the application and together form the logic of the application. E & F are the ORM, where F is the sql engine actually using the factory. .NET fullIn .NET full, the factory is registered in machine.config or app/web.config, and nothing is done further: F requests factory from system and uses that to use ADO.NET classes. CoreFX, this proposalIn CoreFX, the most ideal way to make this work is to do registration in A. This would mean all other code further down the road (B->F) can be the same as on .NET full: in F on CoreFX, the same code is used to obtain the factory to use ADO.NET classes. IMHO this also means that CoreFX specific code is located in A (and that's fine, it's a CoreFX app!), and all other code (B->F) is not relying on CoreFX specific APIs: they use the same API on .NET full as on CoreFX: Only F has to obtain the factory and does so using an API that's shared between .NET full and CoreFX. |
So this is all about compatibility and nothing more (Like a better API design?). If so, then it looks fine to me. |
@divega That was my idea too hence my attempt to implement it so it could be merged in 2.0.0 and my struggles to get the tests running outside of .netstandard :). I think a bit of miscommunication resulted in the confusion that's currently surrounding this topic. If I can get some tests running on .netcore the code should be ready to go (but that's currently not working due to tooling issues) |
@FransBouma I was under the impression that latest info should have unblocked you. Is that not the case? |
@karelz to be honest I haven't tried as it was scheduled to be .netstandard 2.1 anyway so I thought it would be better to wait till 2.0.0 hits and then take another stab at it, as tooling would be mature by then (and it wouldn't be merged before that anyway ;)). Will try later today :) |
Tooling won't mature on its own sadly :(. My latest understanding was that you were blocked on lack of documentation how to add netcoreapp-only APIs. That should have been slightly better after @tannergooding kindly added it to docs notes (the docs are likely in wrong section, because I pointed Tanner to the wrong section - it should be in the one above, but the principle is the same, there is overlap between the 2 anyway) Note: master branch is already opened for 2.1 work. Last integration into release/2.0.0 branch from master happened last week. We already changed master branch versions to 2.1. |
@karelz I've tried that to the letter, but it still doesn't allow me to compile the tests inside vs2017 ('DbProviderFactories is unknown'), no matter what I do. It might be I need 15.3 preview installed (which I haven't and can't it's a production system I need) but no-one has been able to answer me this. So I don't know how to write software using this. I know it's likely something small I'm overlooking, but what that is, I have no idea. I've committed the changes I've made to the PR, so others can see what I have and what might be the problem, I am out of options. It's really simple:
At the moment it's, sorry, chaos. The fragments are scattered, they're there but not known to people outside MS like myself, and it's a puzzle in which order which fragments have to be read/used and which ones are outdated. That's why I thought it would be better to wait till things are more settled down so an outsider like me, who doesn't know details of the scripted system in place to make it all compile and work, can simply put code into some files, configure things based on easy step lists and contribute without having to do the trial/error dance. As it's at the moment no fun at all. |
I understand your frustration, let's look into it deeper. |
@karelz no my standards are pretty low in this regard. I wrote a complete linq provider without any docs from MS on what Queryable was at the time, 9 months of full time trial/error. That's why they call it software engineering, right? As a hobby I even reverse engineer 3D game engines to add camera tools to manipulate them at runtime through dll injection and assembler/C++. I don't care if there are no docs, but at least that there is some idea what to do. Like I said before to you, I've spent almost a week full time on this, with 90% fighting with tools. I did that because I didn't want to give up easily. So no, that's definitely not it. Here I simply don't know what to do anymore to make it work. I did read every doc I could find on this, as I didn't want to come across as this arrogant dev who thinks he knows best and runs in without RTFM. But here I am, frustrated after spending all this time on this, with no way to compile a simple test, left alone debug it. (Yes, I learned from an issue in the xunit add-in repo for vscode of all places that one could debug a test by adding a while(!Debugger.Attached()) to the test to make you debug it, I even tried vscode but the xunit add in can't (yet) deal with conditional includes in the csproj.)
I must say this didn't get down well with me, at all. How do I have to read this other than that it looks like I'm some pampered vs oriented dev who drags/n/drops his code together? |
@FransBouma I didn't try to question your technical capabilities, experience or willingness to push through. I fully trust that you are super-b engineer, you proved that already before. I was honestly trying to find what is different on your approach than what others went through. It doesn't speak anything about you, it speaks about what tools you used and how and that somehow that went different path than others. I totally expect that all this is just some gap in our engineering experience, I just don't know where yet. I definitely appreciate your time and patience so far, you invested more than 1 week full time on making it work. It is way above anything I would expect from you or anyone else to do. And believe me, we appreciate your effort. It won't be total waste of time as it will help us shape docs for future contributors. |
No, I agree with @FransBouma. The practices that have become obvious to regular contributors are still really confusing and time consuming to grep, even when you move to doing different kinds of contribution like API addition. Running and debugging tests is tedious and I couldn't always get it to work. Just a data point. Every time I have asked for some straightforward docs on each type of procedure, I hear that your team practices are evolving too fast and people don't have time to document it. (And then I am welcomed to document your practices, even though I'm never going to have the full picture even when I figure out everything necessary for my contribution.) It is essential to maintain and evolve crystal clear and comprehensive step-by-step contribution instructions along with the actual projects and build scripts. Having that makes your project more attractive to outside contributors and is more considerate of their time, to put it bluntly. I've contributed to a number of projects and this is the single biggest thing you notice about each project. Sure, I'll contribute again because are you kidding, this is the BCL! =) It's worth the effort. |
Thanks for feedback @jnm2! It helps to know others are/were facing similar problems. It will help us bump priority of the docs. Good new contrib docs is something I have on my todo list for quite a while. I hope that I will be able to ask someone on the team to help improve them during June. I believe we passed the point of "we are evolving too fast to have docs" in January. Our infra is pretty decent now and I don't think we can afford that excuse anymore. VS integration for contributors is lacking, we know that. At minimum we should call out huge warning in the docs. Ideally we would fix VS and then react to every single report of things not working. cc @weshaggard @ericstj @mellinoe @danmosemsft |
@karelz Thanks for considering! I saw a similar comment here: https://github.com/dotnet/corefx/issues/20325#issuecomment-305127003 |
@jnm2 that feedback is about using .NET Core 2.0 to develop libraries. It is different than the topic here - contributions to the CoreFX repo itself. |
Okay, my bad. I thought ImageSharp was a corefx-like project. |
As one of those 50 new corefx contributors, I don't think I can allow myself to be co-opted in wholehearted support of the way things are. I found corefx very difficult to work on:
For balance, I'll say that my very positive experiences of working on corefx stem from the friendly and helpful attitude of the corefx MS guys on GitHub. I wish I could have @karelz to do first-line support on every OSS project I've worked on, and there were lots of others too... I actually do get it why the framework's own development couldn't start from exactly the same tooling position as end-users will ultimately use, and corefx probably feels the need to outpace the lumbering old donkey which is VS. But corefx is basically only building class libraries, so I don't see why it needs to be so far in left field. However, I can't help thinking that if the corefx team actually had to develop corefx code in the same way as every other .NET library is developed (i.e. in VS, with 3rd-party test runners), then both VS and .NET Core would be better for it. I know this is off-topic here but, genuine question, where would it be on-topic? Can I actually usefully open an issue in corefx's GitHub repo saying (more politely) "The VS .NET Core dev experience is unutterably awful"? The scale of the problem is way beyond filing VS bugs, even if I did fancy exchanging platitudinous snippets with the staff of a Chinese outsourcing company. @jnm2 Has a perfectly valid point - these is absolutely no good reason at this time why the 3rd-party development of a library like ImageSharp should look any different to the MS development of the vast majority of corefx. I'm ready for my telling-off about unprofessionalism now... |
I think I lack this knowledge at the moment. Is there a page you can point me to so I can gain that knowledge and create these projects myself too? I know I can look at other project csprojs, and have done so, but as the machinery uses .props files I don't know how to do this properly. One question I haven't seen answered and which is related is: you do need 15.3 update for vs2017 for this? Thanks
Would you like to share those for others to use? That would be great :) I think it's a matter of once you 'know what to do and have the info bits to navigate around the dark pits of hell' it's easy to contribute and you forget you ever had problems. To get there is a bit of a struggle ;)
Seconded I also agree with the assertions you made regarding xunit and it's... let's say 'less ideal ways of doing things' to stay polite. I've never seen a framework that has one job and do it in such an user unfriendly manner. Almost as equally unpleasant as vi on an SCO unix prompt. Oh... did I just slip off the 'politically correct' path there? ;) |
@FransBouma I don't think I have anything useful to share really - the stuff I worked on was a direct port of netfx code, and so to debug the tests I built them as a netfx project against the original netfx implementation, debugged the tests in netfx land and then went back to corefx. I also never had to create a corefx project from scratch because someone in MS did that, so I can't offer any advice there, other than that I'd probably just copy a simple existing sub-project within corefx and run around changing names, etc. I was using VS2015 at the time with corefx - I wasn't clear about VS2017 support in corefx at that point (earlier this year). I would be very wary of installing the 15.3 preview - it's billed as a safe 'side-by-side' install, but on my machine it trashed all the VS2017 15.2 settings back to defaults which is a huge pain, and the 15.3 preview crashes every 30 seconds anyway, so I didn't find it useful for anything. Here's one of my test-running command lines, though I doubt that's really where you're stuck, and if you're on a development box then there are easier commands to run all the tests in a project which are part of the corefx build system. (Still all command-line though).
If you can get your tooling expectations dialed-back about 25 years (but without the 8.3 filename restriction) then you might find less disappointment. |
@willdean Thanks, any info is good info :) Especially regarding 2015 and 2017 15.3. It tells me the stuff I have in place should be sufficient, and that's not it. If no-one from MS has time to solve it, I'll indeed try to copy an existing project and see whether I can get that running.
heh :) good point. I'll dial it back to the experience with what I used back then haha ;) (vi editing, typing cc or make on command line for compilation, for debugging gdb without source overview, on a 80x24 WYSE terminal) |
For the most part it was fine but there were aspects of using xUnit that I didn't enjoy, also. I don't expect that they're interested in switching but as a data point, while I was contributing I wished I was using NUnit so many times. (This was before I became a member of NUnit, so no disclaimer needed.) |
First, thanks everyone for the feedback - I am ABSOLUTELY interested in this kind of feedback! Now, let's do something about it ... I suggest to move the discussion into separate issue - that is totally appropriate way to discuss "how to contribute to CoreFX". I would suggest to split the discussion into 2 parts - one for contributor experience hassle (incl. VS) (see dotnet/corefx#20570) and second about xUnit (TODO anyone?). |
I suggest in those issues we try to get wider consensus on a prioritized list of improvements. There are lower hanging fruit than xunit, I think. |
I don't think there would be any need to replace xUnit, just have some of the more painful omissions fixed - I think there is some (historical?) connection between xUnit and MS, so there may be some encouragement could be brought to bear there? |
@willdean if there are painful omissions in xUnit, please file a new issue, so that we can understand them and then act on them. Thanks! |
Nice to see I'm not alone in my xunit non-fandom. Here are my issues (from a very cmd-line bare-metal developer when it comes to tests.) Looking forward to that xunit thread... |
* Implements #19826: DbConnection doesn't offer an internal property ProviderFactory * Added test for #19826. Due to project misconfiguration it fails locally. * Removed writeline * Added comment to illustrate the purpose of the property which currently is dead code. * Implementation of DbProviderFactories, ported from NetFx and adjusted to CoreFx See #4571, #19825 and #19826 * Update for project files to get a netcoreapp test set * Changes to project system to get the stuff compiled. Failed. * Various changes: - Updated config/csproj files for netcoreapp for tests. - Properly moved DbProviderFactories file to right location - Added DbProviderFactories type to ref project, split across 2 files (one for netcoreapp specific methods) - Changed SetFactory to ConfigureFactory - Changed generic type argument to normal method parameter - Removed default parameter values and added overloads * Added tests for DbProviderFactories. * Better subclass testing added and more tests added * Moved all hard-coded strings to SR/Strings.resx. Added test for factory retrieval using a connection * Removal of now redundant comment. See: #19885 (review) * Comment correction for bad English * Refactored API a bit, see: dotnet/standard#356 (comment) * Added tests for reworked API. Added tests for wrong type passing * Reverting change in sln file: See #19885 (review) * Almost done implementation using DataTable internal storage. Refactoring now to concurrentdictionary * Final work for #20903 * Corrections of the implementation-> - registrations of assembly type name are now deferred - storage is now a struct, as the typename has to be registrated as well. - corrected a wrong nameof() call. - added tests for the change behavior in RegisterFactory(string, string) * Final implementation * Corrections made to code by request of @saurabh500 * Github for windows didn't commit this file in last commit... :/ * Again correcting component elements. These are automatically inserted so if they're back again, I can't remove them * ... annnnd another component element. Last one I hope * @divega requested changes - Corrected sln file netcoreapp->netstandard - Corrected wording in exception messages. - Component elements are back, I'll leave them now, they get back regardless. - Renamed column name constants to have the 'ColumnName' suffix for more clarity - Made Instance constant be initialized with nameof(Instance) - Added using for System.Reflection so less verbose code * More @divega requested changes - factored out a clone into its own method (GetProviderTypeFromTypeName) - Reverted 'nameof(Instance)' to string, and renamed field - Removed redundant null/length check in ctor of ProviderRegistration ctor * Last nit removal for @divega
I have seen no info about this with respect to .NET Core, and it's a crucial element for (micro) ORMs and data-access libraries. The .NET full API for DbProviderFactory contains usage of DataTable, so I recon that's not going to be ported anytime soon, but there's a bigger problem: most provider factories are defined in machine.config, which with 'bcl per app' isn't going to fly anymore.
Have there been any design thoughts on this? Will .NET core have DbProviderFactory machinery so an ORM can obtain the factory from a central API (I couldn't find the API of .NET full in corefx at the moment), and create ADO.NET elements using that factory?
Or is this in limbo just like DataTable (https://github.com/dotnet/corefx/issues/1039)
The text was updated successfully, but these errors were encountered: