btw Chris, using the overload with Action<string> and turning off the create and drop flags you can catch any single line of the script and do what you need with it ;)
2010/3/5 Chris Collier <[email protected]> > OK. I can submit a new patch that uses an overload instead of the > config setting, and one for the the documentation. The method I > mentioned in in DriverBase is private, so no API concerns. I'll send > a patch for that too, it's just one line. You can take a look. > > The only problem now is my Jira account (mentioned in a separate > post). Sorry for the inconvenience on that; don't know what happened. > > Thanks for the help. > > BTW, just wanted to say thanks for NH too! NHibernate is a huge part > of my work these days. I work for a consulting firm (we build custom > business app) and we now have four projects that rely on it for > persistence. I have now taught many developers how to use it. I'm a > huge believer. We're very excited about 3.0, the new Linq provider, > etc. > > Chris > > > On Mar 5, 12:27 am, Fabio Maulo <[email protected]> wrote: > > for time-out I think is better to use an overload with the specific > time-out > > only for SchemaExport. > > for the other matter of point 1 I would see what you are proposing and > then > > try to understand why. > > > > There are times where we prefer don't change the signature of a public > > method and avoid a breaking change, take care. > > The most big breaking change was 1.2->2.0 (renaming of some classes, > methods > > and namespace); if the reason of breaking change is not so strong we > prefer > > avoid it. > > > > About the documentation: thanks, a patch is always welcome > > > > 2010/3/5 Chris Collier <[email protected]> > > > > > > > > > > > > > Hi Fabio, > > > > > There are three parts, two fixes and some low priority cleanups I > > > could optionally do while in the code: > > > > > 1. Currently, the command_timeout configuration setting has no effect > > > when using SchemaExport. The patch I submitted to Jira: > > >http://216.121.112.228/browse/NH-2126, fixes this. I think > > > SchemaExport should look at this setting, because otherwise there is > > > no way to set commandTimeout on the IDbCommands. AFAIK, it has to be > > > set on the IDbCommand object directly, which there is no access to > > > outside SchemaExport. Users may or may not want a different timeout > > > setting for DB creation than for queries, but this at least gives them > > > the option of setting it at all for DB creation. I guess another > > > option would be to expose another public Execute method that takes a > > > caller-supplied IDbCommand rather than an IDbConnection. > > > > > We need it in our project because we're abusing <database-object> in > > > HBMs to execute long-running ETL stored procs to populate our database > > > at the end of the DB creation. One of the ETL operations takes > > > approx. 45 seconds, which is longer than the 30-second default > > > commandTimeout value. > > > > > 2. I don't know if the same problem exists in SchemaUpdate, but I can > > > submit a patch for that if so, since I'm now familiar with that part > > > of the code. > > > > > 3. Cleanups: First, the two public Execute(bool, bool, bool, > > > IDbCommand, TexWriter) methods in SchemaExport have comments. But the > > > two public Execute(Action<string>, bool, bool, IDbConnection, > > > TextWriter) methods do not. I find this a little misleading and would > > > comment the latter two. > > > > > Second, when debugging through, I noticed that in Driver/ > > > DriverBase.cs, the SetCommandTimeout method (starting on line 139) > > > takes two parameters, but the second parameter (the timeout value) is > > > never used inside the method. A member variable is used instead. So > > > I propose to remove that second, unused parameter from the method > > > signature. > > > > > The cleanups (1) some comments, and (2) removing an unused method > > > parameter, are pretty clearly not risky changes. > > > > > I figure fixing anything I notice is useful, even if small or not > > > related to an actual bug. > > > > > I hope this clarifies and is helpful. > > > > > Chris > > > > > On Mar 4, 10:21 pm, Fabio Maulo <[email protected]> wrote: > > > > Which is exactly the target of what you want do ? > > > > Which are improvements or problems you are trying to solve ? > > > > > > I didn't see any problem or particular issue there. > > > > > > 2010/3/5 Chris Collier <[email protected]> > > > > > > > OK, will do. I'll proceed that way. What's the best form to > provide > > > > > the test cases in? Patch to appropriate test fixture class in > > > > > NHibernate.Test project? > > > > > > > Thanks, > > > > > Chris > > > > > > > On Mar 4, 6:07 pm, "Richard Brown \(gmail\)" < > [email protected]> > > > > > wrote: > > > > > > Hi Chris, > > > > > > > > Obviously nobody can guarantee that a patch will be accepted. > Your > > > best > > > > > bet > > > > > > is to do address each (ideally small) issue one at a time, and > > > provide > > > > > test > > > > > > cases with all patches. > > > > > > > > If there isn't actually a bug, then I suspect more detail would > be > > > > > helpful. > > > > > > Probably as an improvement in JIRA. > > > > > > > > Cheers, > > > > > > Richard > > > > > > > > -------------------------------------------------- > > > > > > From: "Chris Collier" <[email protected]> > > > > > > Sent: Thursday, March 04, 2010 9:43 PM > > > > > > To: "nhibernate-development" < > > > [email protected]> > > > > > > Subject: [nhibernate-development] hbm2ddl patch; proposal for > more > > > > > > comprehensive work > > > > > > > > > I recently submitted this patch: > > >http://216.121.112.228/browse/NH-2126, > > > > > > > which I needed for work on the product my team is building > using > > > NH. > > > > > > > In getting into the code, it looks like the hbm2ddl code could > use > > > a > > > > > > > more complete fix to the issue I addressed: IDbCommand objects > not > > > > > > > being created using the same mechanisms the rest of NH uses > (when > > > > > > > appropriate). > > > > > > > > > In my opinion, the way the Execute(...) methods are chained > could > > > use > > > > > > > some cleaning up as well. > > > > > > > > > I'd like to invest some time in giving some TLC to the hbm2ddl > code > > > -- > > > > > > > does anyone support this effort, i.e., if I do it, assuming > it's > > > > > > > quality code, what are the chances that these hbm2ddl cleanups > > > would > > > > > > > be considered a worthwhile effort for 3.0? I can give more > detail > > > > > > > about my planned changes if wanted. Please advise. > > > > > > > > > Thanks > > > > > > > Chris > > > > > > -- > > > > Fabio Maulo > > > > -- > > Fabio Maulo > -- Fabio Maulo
