include_sql_query requests unuseful parameters for CSV database driver

[ikedas]

(This issue was separated from the issue #1434.)

Version

At least 6.2.48 and later.

Installation method

Any.

Expected behavior

CSV driver requests only necessary parameters, which are currently db_type and f_dir.

Actual behavior

According to a report, Sympa won't let you define an include_sql_query without db_name, db_host and db_user.

Steps to reproduce

See "Steps to reproduce" in #1434 .

Additional information

Some parameters are inconsistent in usage across the various contexts of configurations. Correction will be suggested in subsequent comment.

[ikedas]

Current situation and suggestion

Current situation (at 6.2.68)

Parameter usage in sympa.conf described in the manual:

	MySQL	Oracle	PostgreSQL	SQLite	ODBC (undocumented)*
db_type	required	required	required	required	required
db_name	required	required	required	required	required
db_host	required	(optional)	required	-	-
db_port	(optional)	(optional)	(optional)	-	-
db_user	required	required	required	-	required
db_passwd	(optional)	required	(optional)	-	required
db_env	-	required	(available, but unuseful)	-	-
db_timeout	-	-	-	(optional)	-
db_options	(optional)	(optional)	(optional)	-	-

* According to parameter usage in Sympa::DatabaseDriver::ODBC.

Parameter usage for the other SQL driver:

	CSV *
db_type	required
f_dir	required
db_options	(optional)

* According to parameter usage in Sympa::DatabaseDriver::CSV.

Parameter definition in configuration schema (form 'sympa.conf' and data source definitions):

	db_* in sympa.conf	include_sql_*.db_*
db_type	required	required
db_name	omittable*	required
db_host	(optional)	(optional)
db_port	(optional)	(optional)
db_user	(optional)	required
db_passwd	(optional)	(optional)
db_env	(optional)	(optional)
db_timeout	(optional)	-
db_options	(optional)	(optional)
f_dir	-	(optional)

* "sympa" by default.

Suggestion

Corrections below are suggested:

1. The parameter f_dir specific to CSV driver should be renamed to db_name, and therefore db_name should be required parameter not according to db type.
2. db_user (and maybe also db_host as some reported) shouldn't be the required parameter in include_sql_* paragraph: Particular type of driver may require them.

[salaun-urennes1]

I agree with your proposal.

[racke]

Yes, makes lots of sense to rename f_dir to db_name. 👍

[ikedas]

Please check the PR above.
(For 6.2.48 please check the commit bcc1189 ).