We recently made specifying the database server version mandatory for Pomelo.
To supply the server version, we made it a mandatory parameter of the .UseMySql() method. The server version is encapsulated in a couple of classes (e.g. MySqlServerVersion or MariaDbServerVersion).
The question now is, what namespace to put those classes in. Should they be in the Microsoft.EntityFrameworkCore namespace, like the UseMySql() methods, or in the Microsoft.EntityFrameworkCore.Infrastructure namespace, like MySqlDbContextOptionsBuilder, or in our own namespace like Pomelo.EntityFrameworkCore.MySql.Infrastructure?
Users would likely import the namespace, since they will usually need to instantiate classes from it.
The same question applies basically also to types, that are used in our MySqlDbContextOptionsBuilder methods. While the class itself resides in Microsoft.EntityFrameworkCore.Infrastructure, in which namespace should we put classes, that will be used in some of the method calls of the class? Microsoft.EntityFrameworkCore, Microsoft.EntityFrameworkCore.Infrastructure or Pomelo.EntityFrameworkCore.MySql.Infrastructure?
My suggestion would be to add in Microsoft.EntityFrameworkCore. We have added enums like QueryTrackingBehavior or QuerySplittingBehavior in that namespace, which are similarly used in further config of UseProvider methods. Especially if they are mandatory then asking user to import additional namespace to finish configuration is a papercut.
Would love to hear what others in team thinks.
We are also currently thinking about putting the classes used for the mandatory parameter in the Microsoft.EntityFrameworkCore namespace, but putting the classes used only in the MySqlDbContextOptionsBuilder methods in the Pomelo.EntityFrameworkCore.MySql.Infrastructure namespace, because they are not prefixed with MySql or MariaDb and we don't want people that use multiple providers do start wondering, which provider they belong to.
@lauxjpn As Smit indicated, it usually pivots on whether or not the mainline scenario will require another using. A using for the namespace is going to be required for enum literals. So if every call to UseMySql needs an enum literal, then I would put those enums in Microsoft.EntityFrameworkCore.
Sounds to me like your suggested approach matches this.
Thanks, then I will go with that for now, so we can get most of the (compile-time) breaking changes that will target most Pomelo users out today in one big swoop.
@lauxjpn Btw, I like the breaking change to make the version explicit. 馃憤
It's an interesting approach. In Npgsql I also allow the version to be set, but it's optional and default to a reasonably recent version - I haven't received any complaints so far. But there are advantages to just forcing users to always specify this.
@roji My impression is that this has always been a bigger problem for MySQL. Originally the MySQL provider would query the database to get the version.
Up until EF Core 3.0 we basically got away with users not specifying the version. But since query generation got way more complex then, we needed to rely more on newer features, that were not available in older but still officially supported versions of MySQL and MariaDB.
And then there is the raising amount of differences in details between MySQL and MariaDB features that got added independently in both implementations, or even just in one of the forks.
We are regularly getting the same issues reported, that are just a result of people not specifying their actual server version, which then leads to the provider assuming, that they are using the latest available version of MySQL when they are actually not, resulting in syntax errors etc.
So now, users have to specify their version, which forces them to make an active decision about what version to support. They can also just explicitly use the latest supported MySQL or the latest supported MariaDB version and they can also just make a call to auto detect the server version with an additional database roundtrip, if they want to.
The general feedback seems to be good so far (see https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/issues/1088#issuecomment-726047863 and following comments).