Bug 32687 - About may list version of SQL client in container, not actual server
Summary: About may list version of SQL client in container, not actual server
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: About (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-01-20 08:13 UTC by Marcel de Rooy
Modified: 2023-12-28 20:42 UTC (History)
3 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:
23.05.00,22.11.04
Circulation function:

Attachments
Bug 32687: Get server version in Context::get_versions (3.17 KB, patch)
2023-01-20 09:40 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 32687: Get server version in Context::get_versions (3.18 KB, patch)
2023-01-21 18:50 UTC, David Nind 		Details | Diff | Splinter Review
Bug 32687: Get server version in Context::get_versions (3.26 KB, patch)
2023-02-23 10:03 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2023-01-20 08:13:19 UTC 
CLIENT
root@master:/usr/share/koha# mysql -V
mysql  Ver 15.1 Distrib 10.5.18-MariaDB, for debian-linux-gnu (x86_64) using  EditLine wrapper

SERVER
root@mastersql:/app# mysql -V
mysql  Ver 15.1 Distrib 10.5.17-MariaDB, for debian-linux-gnu (x86_64) using readline 5.2

About lists 10.5.18. The server is still on 17..
Comment 1 Marcel de Rooy 2023-01-20 09:40:42 UTC 
Created attachment 145504 [details] [review]
Bug 32687: Get server version in Context::get_versions

Test plan:
Run t/db_dependent/Context.t
Check About/Server information/MySQL version.
Comment 2 David Nind 2023-01-21 18:50:23 UTC 
Created attachment 145563 [details] [review]
Bug 32687: Get server version in Context::get_versions

Test plan:
Run t/db_dependent/Context.t
Check About/Server information/MySQL version.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2023-01-21 18:55:51 UTC 
Testing notes using KTD:

1. About page:
   - Before:  MySQL version: mysql Ver 15.1 Distrib 10.5.18-MariaDB, for debian-linux-gnu (x86_64) using EditLine wrapper 
   - After: MySQL version: 10.5.18-MariaDB-1:10.5.18+maria~ubu2004

2. From the command line in KTD:

root@kohadevbox:koha$ mysql -V
mysql  Ver 15.1 Distrib 10.5.18-MariaDB, for debian-linux-gnu (x86_64) using  EditLine wrapper

3. From the server - select version() and status queries (mysql-kohadev kohadev):

MariaDB [koha_kohadev]> select version();
+-----------------------------------------+
| version()                               |
+-----------------------------------------+
| 10.5.18-MariaDB-1:10.5.18+maria~ubu2004 |
+-----------------------------------------+

MariaDB [koha_kohadev]> status;
--------------
mysql  Ver 15.1 Distrib 10.5.18-MariaDB, for debian-linux-gnu (x86_64) using  EditLine wrapper

...
Server:                 MariaDB
Server version:         10.5.18-MariaDB-1:10.5.18+maria~ubu2004 mariadb.org binary distribution
...

--------------
Comment 4 Jonathan Druart 2023-02-23 09:24:27 UTC 
+        $mysqlVersion          ||= `mysql -V`; # fallback to sql client version?

I would remove this. Why keep it?
Comment 5 Marcel de Rooy 2023-02-23 09:50:45 UTC 
(In reply to Jonathan Druart from comment #4)
> +        $mysqlVersion          ||= `mysql -V`; # fallback to sql client
> version?
> 
> I would remove this. Why keep it?

Theoretical fallback. Would leave it there.
Comment 6 Jonathan Druart 2023-02-23 10:01:35 UTC 
(In reply to Marcel de Rooy from comment #5)
> (In reply to Jonathan Druart from comment #4)
> > +        $mysqlVersion          ||= `mysql -V`; # fallback to sql client
> > version?
> > 
> > I would remove this. Why keep it?
> 
> Theoretical fallback. Would leave it there.

But this patch shows it does not return the correct value, and if DBI->get_info does not return something, we can expect bad things to happen.
Comment 7 Jonathan Druart 2023-02-23 10:03:29 UTC 
Created attachment 147211 [details] [review]
Bug 32687: Get server version in Context::get_versions

Test plan:
Run t/db_dependent/Context.t
Check About/Server information/MySQL version.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2023-02-23 10:03:55 UTC 
Letting RM takes the decision.
Comment 9 Tomás Cohen Arazi (tcohen) 2023-02-24 20:38:53 UTC 
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 10 Matt Blenkinsop 2023-02-28 17:08:41 UTC 
Nice work everyone!

Pushed to stable for 22.11.x
Comment 11 Lucas Gass (lukeg) 2023-03-08 14:01:16 UTC 
Doesn't apply clean in 22.05.x, if needed please rebase.