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 (vote)
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


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

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 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 2023-03-08 14:01:16 UTC
Doesn't apply clean in 22.05.x, if needed please rebase.