Alan DeKok said:

Directories are cheap.

sql/templates/database-thingy.conf sql/database/thingy.conf -> ../../templates/..... sql/thingy/database.conf -> ../../templates/...

Of course, that would also simplify configuration, so in radiusd.conf we can have ... db_dialect = mysql ... then in things like sql.conf and sqlippool.conf, instead of having stuff like ... ################################################################ ## Uncomment the appropriate config file for your SQL dialect ## ################################################################ # $INCLUDE ${confdir}/sql/mysql-ippool-dialup.conf $INCLUDE ${confdir}/sql/postgresql-ippool-dialup.conf ... it just becomes ... $INCLUDE ${confdir}/sql/${db_dialect}/sqlippool.conf ... no need to fart around editing the INCLUDE lines.

Alan DeKok.

-- hugh

Hugh Messenger wrote:

Of course, that would also simplify configuration, so in radiusd.conf we can have ...

db_dialect = mysql

It's a lot easier to understand, and a lot easier to manage. "Change db_dialect to your DB and everything will Just Work" is a good thing to have.

... it just becomes ...

$INCLUDE ${confdir}/sql/${db_dialect}/sqlippool.conf

... no need to fart around editing the INCLUDE lines.

Much, much, better. We should do this for 2.0, not for 1.1.7 of course... Alan DeKok.

On Fri 20 Jul 2007, Hugh Messenger wrote:

Peter Nixon said:

...

Hi Guys

Over the last few months I have been trying to clean up the SQL config files ready for the 2.0 release. We currently have the following in raddb/sql/

mssql-dialup.conf mysql-dialup.conf mysql-ippool-dialup.conf mysql-sqlcounter.conf oracle-dialup.conf postgresql-dialup.conf postgresql-ippool-dialup.conf postgresql-sqlcounter.conf postgresql-voip-postpaid.conf

The question is, is it best to do it this way with the database name first, or should we put the database name last, just before the .conf. For example "raddb/sql/counter-mysql.conf"

I kinda like it the way it is, but could live with it either way.

BTW, why the '-dialup'? Doesn't really matter, but it is a little misleading, as there's nothing specific to dialup in there.

Well, there kinda is. Compare with postgresql-voip-postpaid.conf :-) I like Alan's suggestion though.. Cheers -- Peter Nixon http://peternixon.net/

Hugh Messenger wrote:

I also like Alan's suggestion of moving the schemas into the ./sql hierarchy as well, makes more sense than keeping them tucked away in docs/examples.

The schemas are no use to the FreeRADIUS server itself. They aren't a part of the config, therefore I see these files more like documentation than configuration. -- Nicolas Baradakis

Hugh Messenger wrote:

So how about something like:

./sql/mysql/queries/sql.conf

That's a few too many directories for me. I've just done: sql/mysql/schema.sql sql/mysql/*.conf Hmm... maybe I should have made "dialup.sql", which means adding a "ippool.sql" schema would make sense.

Etc etc. And yes, I'm aware that currently the radippool table is defined in the main foo.sql schema. IMHO it should be in its own file, again named after the module.

Yes, I agree.

The above structure clearly shows which dialect it is, if it's a query config or a schema, and which module it belongs to.

Also see my comment in a previous post about simplifying the top level sql*.conf files by adding a 'db_dialect' config variable in radiusd.conf.

Done. Please check out CVS head and test. Alan DeKok.

I know I already mentioned this several times, but ... can't we call the files after the modules they belong to (minus the rlm_):

sql/mysql/sql.conf sql/mysql/sql.sql sql/mysql/sqlippool.conf sql/mysql/sqlippool.sql

... so at a glance we know sql.* belongs to rlm_sql, sqlippool.* belongs to rlm_sqlippool, etc.

Calling the base rlm_sql stuff 'dialup' is a little confusing ... for instance, I don't have a single dialup going though FR (yet, busy migrating from Funk/Juniper), but I make extensive use of rlm_sql.

Point taken. I am by no means wedded to the name dialup, but as I mentioned previously I wanted to have the ability to have different configs for different types of installations. I was planning to contribute a fully virtualised config shortly which separates everything by callingstationid for example. (And we already have the voip specific config for postgresql) Maybe we should call it sql/mysql/default.conf etc What do you think?

And having ippool.* tends to suggest it has something to do with the 'ippool' module rather than the 'sqlippool' module.

I think the sql/ directory gives away the fact that its an sql related module :-)

A minor point, I know, but anything that helps reduce potential confusion is a Good Thing, IMHO.

Agreed, but is this actually confusing? The top of the file lists: # FreeRADIUS rlm_sqlippool SQL Queries for the MySQL Dialect I think any confusion should quickly pass... -- Peter Nixon http://peternixon.net/

On Fri 20 Jul 2007, Hugh Messenger wrote:

...

I like Alan's suggestion though..

Yup, me too. But I would like to use the module name as the base name for the file, wherever possible.

I also like Alan's suggestion of moving the schemas into the ./sql hierarchy as well, makes more sense than keeping them tucked away in docs/examples.

Agreed.

I'm not wild about using symlinks, though.

Me neither

So how about something like:

./sql/mysql/queries/sql.conf ./sql/mysql/queries/sqlippool.conf ./sql/mysql/queries/sqlcounter.conf ./sql/mysql/schema/sql.sql ./sql/mysql/schema/sqlippool.sql ./sql/postgresql/queries/sql.conf ./sql/postgresql/queries/sqlippool.conf ./sql/postgresql/queries/sqlcounter.conf ./sql/postgresql/schema/sql.sql ./sql/postgresql/schema/sqlippool.sql

Etc etc. And yes, I'm aware that currently the radippool table is defined in the main foo.sql schema. IMHO it should be in its own file, again named after the module.

Why? What is the benefit of making people import two schema's instead of one? I don't see how having one extra, possibly unused table in the default schema is at all harmfull, and in my opinion the added "cleanliness" does not make up for the extra command every user will have to type when they want to setup sqlippool (Which I believe is going to be a wildly popular module in the near future :-) -- Peter Nixon http://peternixon.net/

On Mon 23 Jul 2007, Alan DeKok wrote:

Peter Nixon wrote:

...

Why? What is the benefit of making people import two schema's instead of one? I don't see how having one extra, possibly unused table in the default schema is at all harmfull, and in my opinion the added "cleanliness" does not make up for the extra command every user will have to type when they want to setup sqlippool (Which I believe is going to be a wildly popular module in the near future :-)

That's what documentation && makefiles are for.

We could have "make install-schema" which installs the schemas. At that point, it doesn't matter if they're in one file, or 10,000 files.

That is actually not as easy as it sounds as every operating system and database has a different method of authenticating to the DB engine and installing the schema. On SUSE at least, password authentication is not enabled by default with PostgreSQL, instead in checks your local IDENT. Therefore, even in a radius user exists in the DB as root or any other user you cannot login as that user. To even create a Postgresql "radius" user, you have to "su - postgres ; createuser radius" as you cannot connect to the db as the "postgres" DB super user unless you are logged into the system as the "postgres" system user. I am also not sure that we want to be automagically changing the postgresql config file and restarting it. IMHO these operations are best left to the DBA, however making those operations as easy as possible is obviously a good thing. Cheers -- Peter Nixon http://peternixon.net/