Skip to content

NH-4026 - Upgrade Firebird driver and use server #639

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.

Sign up for GitHub

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

Merged
merged 17 commits into from
Jun 10, 2017
Merged

NH-4026 - Upgrade Firebird driver and use server #639

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7c8aa5a
NH-4026 - upgrading Firebird driver and using server instead of embed…
fredericDelaporte Jun 2, 2017
4d36a19
NH-4026 - A connection string parameter is missing for addressing Fir…
fredericDelaporte Jun 2, 2017
fd84733
NH-4026 - some other changes just to try, although my laptop does not…
fredericDelaporte Jun 3, 2017
fe0e8ac
NH-4026 - fix the test database setup (or at least tries to).
fredericDelaporte Jun 3, 2017
728ede7
NH-4026 - let the TestDatabaseSetup create the db.
fredericDelaporte Jun 3, 2017
704c122
NH-4026 - trying to document the relevant things I have done to final…
fredericDelaporte Jun 3, 2017
96ac0d3
NH-4026 - Firebird locks table till connection are actually closed, n…
fredericDelaporte Jun 4, 2017
ab0410c
NH-4026 - some more explanations.
fredericDelaporte Jun 4, 2017
da88449
Revert "NH-4026 - some more explanations."
fredericDelaporte Jun 4, 2017
d75f7ef
NH-4026 - some more explanations.
fredericDelaporte Jun 4, 2017
8b832cf
NH-4026 - fixing remaining failing tests.
fredericDelaporte Jun 5, 2017
e794efe
NH-4026 - remove ignoring previously failed.
fredericDelaporte Jun 5, 2017
d04fafa
NH-4026 - fix a typo in default test config file. Align naming on oth…
fredericDelaporte Jun 6, 2017
3a5395e
NH-4026 - disabling forced writes and simplifying setup.
fredericDelaporte Jun 7, 2017
d71f510
NH-4026 - Updating instruction file
fredericDelaporte Jun 7, 2017
b42cfb3
NH-4026 - relocating the db, and ensuring forced write option by drop…
fredericDelaporte Jun 7, 2017
907b5cf
NH-4026 - removing explicit dependency on D: from TestDatabaseSetup.
fredericDelaporte Jun 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/teamcity/firebird/firebird_installation.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Installation steps for Firebird for NH TeamCity:
5. Go into Firebird folder (c:\program files\firebird\) and create a folder named Data;
6. Go in Firebird installation directory and open databases.conf;
7. Add in "Live Databases" section:
nhibernate = C:\Program Files\Firebird\Data\nhibernate.fdb
nhibernate = D:\SqlData\Firebird\nhibernate.fdb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make it local?

Copy link
Member Author

@fredericDelaporte fredericDelaporte Jun 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local to the database server? Would be on C:, which is not local to the host. Local to the NHibernate test client? The path seems not frozen, only embedded mode can do that, where the client is the server too. I think Firebird mainstream usage is as an actual server, not as embedded mode. So I would rather test it as an actual server. And as showcase by Nathan branch, Firebird v3 embedded mode seems to need a new dialect.

When using a database server, does it really make sens to locate the database relatively to the client? That should be the server choice, not the client. Currently this is done with a configured alias in FireBird_3_0\databases.conf file, which decides where to put the file for the alias nhibernate.

8. Open firebird.conf;
9. Ensure AuthClient, AuthServer and UserManager are set to Srp only:
AuthServer = Srp
Expand All @@ -18,7 +18,9 @@ UserManager = Srp
WireCrypt = Enabled
11. Restart Firebird service.

The TestDatabaseSetup will create the database on the Teamcity agent.
The TestDatabaseSetup will create the database on the Teamcity agent. It will create the folder
D:\SqlData\Firebird if it is missing. Firebird is particularly sensitive to disk performances,
and D: is supposed to be local to the host, thus this choice.
For manual testing, take care of not creating it with inadequate acl on the file. This may happen
if you use ISQL with a connection string causing it to create it in embedded mode, without actually
using the server. Prefixing your path with "localhost:" should avoid that.
Expand Down
11 changes: 10 additions & 1 deletion src/NHibernate.TestDatabaseSetup/TestDatabaseSetup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,17 @@ private static void SetupSqlServerOdbc(Cfg.Configuration cfg)

private static void SetupFirebird(Cfg.Configuration cfg)
{
Directory.CreateDirectory(@"D:\SqlData\Firebird");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like references to a specific folder. I think we need to use a local (relative) folder. Building with VS and NAnt+MSBuild will produce different results. When building with NAnt the output folder would be /build/version/net40 (or something like this). Building with VS will produce tests assemblies in different folders (/src/NHibernate.Test/bin/Debug-2.0/, /src/NHibernate.TestDatabaseSetup/bin/Debug-2.0/, etc). We need to choose some common folder, probably "curent-test-configuration", or a repository root.

I think we shall/should parse connection string to find where the file sits.

Copy link
Member Author

@fredericDelaporte fredericDelaporte Jun 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The connection string does currently only hold the alias nhibernate. The actual path is in databases.conf file, in Firebird installation directory. Yes this CreateDirectory code line is a crutch for accounting the possible loss of D: content. The server do not create the directory itself. We may instead just drop the db on the root, D:\, avoiding this above line of code.

When using a database server, it does not really make sens for me to locate the database relatively to the client, that is usually the server choice. Firebird allows to specify the database location from the connection string, which is a strange model for a server, not at all common. So we may do that anyway, by detecting where the test assemblies are then tweaking the connection string for telling the server where to put the file. But that would depart from what is and can be done for other actual servers like Oracle, Sql-Server, PostgreSql, which gives no option to the client about where to locate the database.
And we would have to do that both in TestDatabaseSetup and the test project.

Otherwise we should go back to testing Firebird in embedded mode, with all its additional binaries and apparently some new behaviors in v3 requiring a dedicated dialect. (There are seven hundred failing tests when running in embedded mode with Firebird v3, most solved by redefining the boolean type as have done Nathan. I have not yet investigated the other cases, since I do not consider we should test Firebird as an embedded server.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem, that people might not necessarily have a "D:" drive, and this would fail. I usually use TestDb to create a fresh copy of a database (including Firebird) myself.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I may just locate the file on d:\ root if that prove to be any better than putting it within Firebird installation folder. That would avoid that line. Actually I was seeing TestDatabaseSetup as a Teamcity exclusive thing, managing myself my own local db. So I would avoid to do such changes now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still be executed if you run from ShowBuildMenu (release option, for ex)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I have missed that, sorry.

var connStr = cfg.Properties[Cfg.Environment.ConnectionString];
FbConnection.CreateDatabase(connStr, forcedWrites:false, overwrite:true);
try
{
FbConnection.DropDatabase(connStr);
}
catch (Exception e)
{
Console.WriteLine(e);
}
FbConnection.CreateDatabase(connStr, forcedWrites:false);
}

private static void SetupSqlServerCe(Cfg.Configuration cfg)
Expand Down