Summary: | Add ability to run misc/devel/update_dbic_class_files.pl without passing parameters by defaulting to koha-conf.xml | ||
---|---|---|---|
Product: | Koha | Reporter: | Julian Maurice <julian.maurice> |
Component: | Command-line Utilities | Assignee: | Julian Maurice <julian.maurice> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | 1joynelson, jonathan.druart, m.de.rooy, martin.renvoize, olli-antti.kivilahti, robin, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
20.05.00
|
|
Circulation function: | |||
Attachments: |
Bug 21177: Use koha-conf.xml in misc/devel/update_dbix_class_files.pl
Bug 21177: Use koha-conf.xml in misc/devel/update_dbix_class_files.pl Bug 21177: Use koha-conf.xml in misc/devel/update_dbix_class_files.pl |
Description
Julian Maurice
2018-08-08 13:55:47 UTC
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 |