Bug 21177 - Add ability to run misc/devel/update_dbic_class_files.pl without passing parameters by defaulting to koha-conf.xml
Summary: Add ability to run misc/devel/update_dbic_class_files.pl without passing para...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Marcel de Rooy
URL:
Keywords:
: 18229 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-08-08 13:55 UTC by Julian Maurice
Modified: 2020-11-30 21:44 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00


Attachments
Bug 21177: Use koha-conf.xml in misc/devel/update_dbix_class_files.pl (4.45 KB, patch)
2018-08-08 13:56 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 21177: Use koha-conf.xml in misc/devel/update_dbix_class_files.pl (4.53 KB, patch)
2019-11-27 13:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21177: Use koha-conf.xml in misc/devel/update_dbix_class_files.pl (4.62 KB, patch)
2020-02-06 07:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2018-08-08 13:55:47 UTC
It is annoying to have to specify database connection parameters each time DBIx::Class files need to be updated.
Comment 1 Julian Maurice 2018-08-08 13:56:36 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
Comment 2 Jonathan Druart 2018-09-15 18:40:50 UTC
*** Bug 18229 has been marked as a duplicate of this bug. ***
Comment 3 Jonathan Druart 2018-09-15 18:42:26 UTC
Maybe we could even remove the need of the option (--koha-conf) and make it the default?
Comment 4 Julian Maurice 2018-09-15 19:25:03 UTC
(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.
Comment 5 Tomás Cohen Arazi 2018-09-15 20:28:42 UTC
I say do whatever you want, but don't break the kohadevbox use case :-D
You should probably have aliases set for this instead.
Comment 6 Jonathan Druart 2018-09-15 22:32:26 UTC
(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.
Comment 7 Julian Maurice 2018-09-17 07:43:35 UTC
(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 8 Mark Tompsett 2019-07-08 16:05:57 UTC
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.
Comment 9 Jonathan Druart 2019-11-27 13:18:07 UTC
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>
Comment 10 Jonathan Druart 2019-11-27 13:18:46 UTC
(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.
Comment 11 Marcel de Rooy 2019-11-29 07:49:08 UTC
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.
Comment 12 Jonathan Druart 2019-11-29 11:40:54 UTC
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.
Comment 13 Marcel de Rooy 2019-11-29 13:05:01 UTC
(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 ;)
Comment 14 Jonathan Druart 2020-02-05 10:37:57 UTC
(In reply to Marcel de Rooy from comment #13)
> Back to SO for now. Next week things will be better ;)

Ping Marcel? :)
Comment 15 Marcel de Rooy 2020-02-06 07:52:43 UTC
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>
Comment 16 Marcel de Rooy 2020-02-06 07:54:41 UTC
(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 ;)
Comment 17 Martin Renvoize 2020-02-10 10:17:22 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 18 Joy Nelson 2020-03-05 00:36:30 UTC
enhancement not backported to 19.11.x