It is annoying to have to specify database connection parameters each time DBIx::Class files need to be updated.
Created attachment 77574 [details] [review] Bug 21177: Use koha-conf.xml in misc/devel/update_dbix_class_files.pl It is annoying to have to specify database connection parameters each time DBIx::Class files need to be updated. This patch adds a new option --koha-conf that takes an optional <path> which defaults to the value of KOHA_CONF environment variable, and use the database connection parameters found in that file. --db_* options override values from $KOHA_CONF Test plan: 1. Run the script with the same parameters as before the patch and see that it still works. Example: misc/devel/update_dbix_class_files.pl --db_name koha_dev \ --db_user koha --db_pass koha 2. Verify that KOHA_CONF is set and execute: misc/devel/update_dbix_class_files.pl --koha-conf Verify that Koha/Schema files were updated accordingly 3. Execute: misc/devel/update_dbix_class_files.pl --koha-conf \ /path/to/another/koha-conf.xml Verify that Koha/Schema files were updated accordingly
*** Bug 18229 has been marked as a duplicate of this bug. ***
Maybe we could even remove the need of the option (--koha-conf) and make it the default?
(In reply to Jonathan Druart from comment #3) > Maybe we could even remove the need of the option (--koha-conf) and make it > the default? I think that making it the default can potentially break existing workflows. Imagine a script that always run the same command: update_dbix_class_files.pl --db_name $name --db_user $user --db_pass $pass but it also happens to have a $KOHA_CONF with host and port different than localhost/3306 This is probably an extremely rare scenario. I don't know if keeping backward compatibility is worth it in this case.
I say do whatever you want, but don't break the kohadevbox use case :-D You should probably have aliases set for this instead.
(In reply to Julian Maurice from comment #4) > (In reply to Jonathan Druart from comment #3) > > Maybe we could even remove the need of the option (--koha-conf) and make it > > the default? > > I think that making it the default can potentially break existing workflows. What I meant was to use info from $ENV{KOHA_CONF} if called without options.
(In reply to Jonathan Druart from comment #6) > (In reply to Julian Maurice from comment #4) > > (In reply to Jonathan Druart from comment #3) > > > Maybe we could even remove the need of the option (--koha-conf) and make it > > > the default? > > > > I think that making it the default can potentially break existing workflows. > > What I meant was to use info from $ENV{KOHA_CONF} if called without options. Ah so you mean use $KOHA_CONF *only* if no other options is specified ?
Comment on attachment 77574 [details] [review] Bug 21177: Use koha-conf.xml in misc/devel/update_dbix_class_files.pl Review of attachment 77574 [details] [review]: ----------------------------------------------------------------- ::: misc/devel/update_dbix_class_files.pl @@ +61,5 @@ > + print STDERR "Error: KOHA_CONF is not defined\n"; > + exit(1); > + } > + > + $koha_conf ||= $ENV{KOHA_CONF}; I think comment #6 was about putting this kind of line above line 59, and then perhaps just check if ($koha_conf) in 59. This way $koha_conf can default to the environment variable if koha-conf isn't explicitly stated.
Created attachment 95849 [details] [review] Bug 21177: Use koha-conf.xml in misc/devel/update_dbix_class_files.pl It is annoying to have to specify database connection parameters each time DBIx::Class files need to be updated. This patch adds a new option --koha-conf that takes an optional <path> which defaults to the value of KOHA_CONF environment variable, and use the database connection parameters found in that file. --db_* options override values from $KOHA_CONF Test plan: 1. Run the script with the same parameters as before the patch and see that it still works. Example: misc/devel/update_dbix_class_files.pl --db_name koha_dev \ --db_user koha --db_pass koha 2. Verify that KOHA_CONF is set and execute: misc/devel/update_dbix_class_files.pl --koha-conf Verify that Koha/Schema files were updated accordingly 3. Execute: misc/devel/update_dbix_class_files.pl --koha-conf \ /path/to/another/koha-conf.xml Verify that Koha/Schema files were updated accordingly Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to Julian Maurice from comment #7) > (In reply to Jonathan Druart from comment #6) > > (In reply to Julian Maurice from comment #4) > > > (In reply to Jonathan Druart from comment #3) > > > > Maybe we could even remove the need of the option (--koha-conf) and make it > > > > the default? > > > > > > I think that making it the default can potentially break existing workflows. > > > > What I meant was to use info from $ENV{KOHA_CONF} if called without options. > > Ah so you mean use $KOHA_CONF *only* if no other options is specified ? Yes. Not blocker.
database, hostname and port are in koha-conf.xml too It is still "annoying" to enter db_name, hostname, port too. Could be extended to db_driver too. We should remove all parameters coming from koha-conf to be consistent. And when removing the mandatory from db_name, I guess we should add a confirm parameter like other misc scripts.
Marcel, I do not understand. If --koha-conf is passed, all those variables will be retrieved from the config file. Are you suggesting to pick the value from koha-conf by default then overwrite them by the parameters? That's more or less what I suggested in a previous comment. But 1 year after I prefer to let it go as it than to block it again.
(In reply to Jonathan Druart from comment #12) > Marcel, I do not understand. If --koha-conf is passed, all those variables > will be retrieved from the config file. Hmm. Overlooked that probably. > Are you suggesting to pick the value from koha-conf by default then > overwrite them by the parameters? That's more or less what I suggested in a > previous comment. But 1 year after I prefer to let it go as it than to block > it again. Good idea. But ok. Back to SO for now. Next week things will be better ;)
(In reply to Marcel de Rooy from comment #13) > Back to SO for now. Next week things will be better ;) Ping Marcel? :)
Created attachment 98516 [details] [review] Bug 21177: Use koha-conf.xml in misc/devel/update_dbix_class_files.pl It is annoying to have to specify database connection parameters each time DBIx::Class files need to be updated. This patch adds a new option --koha-conf that takes an optional <path> which defaults to the value of KOHA_CONF environment variable, and use the database connection parameters found in that file. --db_* options override values from $KOHA_CONF Test plan: 1. Run the script with the same parameters as before the patch and see that it still works. Example: misc/devel/update_dbix_class_files.pl --db_name koha_dev \ --db_user koha --db_pass koha 2. Verify that KOHA_CONF is set and execute: misc/devel/update_dbix_class_files.pl --koha-conf Verify that Koha/Schema files were updated accordingly 3. Execute: misc/devel/update_dbix_class_files.pl --koha-conf \ /path/to/another/koha-conf.xml Verify that Koha/Schema files were updated accordingly Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Jonathan Druart from comment #14) > (In reply to Marcel de Rooy from comment #13) > > Back to SO for now. Next week things will be better ;) > > Ping Marcel? :) A typical Koha week ;)
Nice work everyone! Pushed to master for 20.05
enhancement not backported to 19.11.x