https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- (In reply to Julian Maurice from comment #1)

+1 for moving Koha config out of zebra config file +1 for not using XML

But -1 for using YAML. I know we already use YAML for everything in Koha, but YAML has a lot of disadvantages

I'm not finding those links particularly persuasive. Using Perl's YAML module, I wasn't able to reproduce a single issue from those links. Plus many of the proposed issues should be easily avoidable anyway.

One concrete problem we had in Koha recently was that some MARC field tags starting with 0 were parsed as octal numbers. (040 becomes 32 but 099 remains 99)

Can you point me to a bug report for that one? That sounds very interesting. I couldn't reproduce that problem based on the description.

I think that this bug might be a good opportunity to consider alternatives (HJSON, JSON5, TOML, ...)

I've never heard of HJSON or JSON5, but I have used TOML a couple times. It was easy to use, and the documentation is good. It looks like libtoml-perl is available in the Debian repositories, so that would be OK. I'm not particularly wedded to YAML. I'd be fine with TOML. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- (In reply to Julian Maurice from comment #3)

Can't remember the exact details and can't reproduce either... it was about the ES mappings file, MARC fields being changed when exporting/re-importing...

Hmm maybe someone else will surface it later.

But the real problem IMO is that YAML is implementation dependent.

That's a good point.

For instance, this document: foo: 040

can be interpreted as { "foo": "040" } or { "foo": 32 } depending on how you read it. (Try this https://onlineyamltools.com/convert-yaml-to-json)

Thanks for that link. That is interesting. However, it seems lazy to write it as 'foo: 040'. Surely it would be safer to explicitly write it as "foo": "040", since we know that they are strings. (That being said, since Perl is weakly typed, I wonder if Perl would try to convert that internally into a number...)

Even in Perl where implicit typing seems to be disabled by default, it can be turned on anywhere in the code ($YAML::Syck::ImplicitTyping = 1), so you can never be sure of what will happen.

In theory, we should know, but I reckon that Koha is big enough that it can be easy to lose track of how the YAML has been set up across the board. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- Julian, after reviewing your comments, I used TOML in a small Perl project, and found it very nice to work with. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- +1 for moving Koha config out of zebra config file +1 for not using XML As for the TOML vs YAML debate, I don't really have a horse in the race. The YAML support in Perl seems to have reached a suitable level of maturity so I feel it's a fairly safe bet really.. but I can understand some of the issues Julian presents. TOML is certainly a bit simpler in scope, and we shouldn't really be using the full scope of YAML in our configs anyway really so I'd suggest we shouldn't really be falling into many of the traps it presents.. How mature is the TOML support in perl... I note David looked at the TOML module (which is pure perl) and hasn't been updated since 2016.. does that mean it's perfect or it's just not maintained? There's also a recent TOML::XS module on cpan.. but will this get packaged and is it likely to be around for a while and maintained. Either way.. I think the splitting out of Zebra config from the config file is a great first step. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 --- Comment #8 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Martin Renvoize from comment #7)

How mature is the TOML support in perl... I note David looked at the TOML module (which is pure perl) and hasn't been updated since 2016.. does that mean it's perfect or it's just not maintained? There's also a recent TOML::XS module on cpan.. but will this get packaged and is it likely to be around for a while and maintained. This looks problematic :/ Maybe it's safer to stick with YAML for now.

Either way.. I think the splitting out of Zebra config from the config file is a great first step. +1

-- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #9)

My idea with bug 14162 was to move this into a file that would be manipulated using a CLI tool, or the UI, and we can maintain with our packaging tools.

I find this idea attractive.

The main problem I faced, and raise here: it would be great to have a way to split the configuration with includes, for example: the ldap config could be a separate file.

That's a good point. I was envisioning configuration broken up to multiple files like that as well, although I figured the key in the main configuration file would be something like "ldap_config_file": "/etc/koha/sites/SITE/ldap_config.yml". I suppose an alternative might be something like... /etc/koha/sites/SITE/conf.d/ldap_config.yml and LDAP code looks for a file called "ldap_config.yml" in a configuration directory pointed to by the main /etc/koha/sites/SITE/koha-conf.yml file? I suppose in both scenarios you need some kind of mechanism for loading the config (either at start-up or on demand...) Food for thought... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Julian Maurice from comment #11)

Yet another alternative would be to replace the main configuration file by environment variables. This is the approach recommended by https://12factor.net/config and can make working with containers (docker/k8s/...) easier.

Of course not everything can be put inside env vars, so it would probably be needed to have env vars that point to files. For instance LDAP_CONFIG_FILE=/etc/koha/sites/SITE/conf.d/ldap_config.yml

Another advantage: no need to worry about the main config file format, or an include mechanism

My vote would be: - Use separate files with expected names (config.yaml, shibboleth.yaml, etc) - No includes - Have C4::Context->config rebuild the 'old structure we already have, and deal with a different new handling on a separate bug (Koha::Config singleton implementation?) - Have a way for C4::Context->config to override any configuration if an env variable exists. e.g. KOHA_DATABASE_NAME should override the relevant entry. My two cents as well -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 --- Comment #14 from David Cook <dcook@prosentient.com.au> --- I've only used k8s once to play around on my desktop, so I don't know it very well, but I think it has more robust methods than AWS ECS for deploying configuration files and managing secrets. But overall I think developing in a way that makes it easier to use containers is a great idea. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 --- Comment #16 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to David Cook from comment #15)

Btw, just sharing an experience using the https://metacpan.org/pod/TOML library.

It takes a TOML Boolean false and converts it into a Perl string that says "false".

Thanks for sharing. The TOML module seems to be the worst implementation available. No release since 2017, only supports v0.4 of TOML spec, and of course this boolean horror :) TOML::Tiny and TOML::XS seems to be better alternatives, both supporting the latest version of the spec, and their latest releases were this year, in august. So, Perl support for TOML has improved since comment 7. And the latest version of TOML::Tiny is packaged in Debian testing, which is promising. Now, is TOML suited for Koha configuration ? That's another question... -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 --- Comment #18 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #17)

my $mq_crud_conf = Koha::Config->relative_file('mq_crud.yml');

Oh but this wouldn't work with Tomas's ideas, which were very good. (In reply to Tomás Cohen Arazi from comment #12)

My vote would be: - Use separate files with expected names (config.yaml, shibboleth.yaml, etc) - No includes - Have C4::Context->config rebuild the 'old structure we already have, and deal with a different new handling on a separate bug (Koha::Config singleton implementation?) - Have a way for C4::Context->config to override any configuration if an env variable exists. e.g. KOHA_DATABASE_NAME should override the relevant entry.

What about using AUTOLOAD? my $mq_crud = Koha::Config->mq_crud(); If the "mq_crud" doesn't exist in an internal data structure, it could then do the Koha::Config->relative_file('mq_crud.yml') or looks for KOHA_MQ_CRUD in an environmental variable to load the configuration from file and then saves it. (This wouldn't use Koha::Cache at all and would require a process restart to refresh configuration like with koha-conf.xml.) The only downside of that would be that you couldn't use an env var override for lower level config... like Koha::Config->mq_crud()->destination; Although you probably could do something clever... where Koha::Config->mq_crud() returns a Koha::Config::Entry object which could use AUTOLOAD or have dynamically created methods during object creation time. There are options... which of course makes it tougher to choose. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|WISHLIST |--- Status|RESOLVED |REOPENED --- Comment #20 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #12)

My vote would be: - Use separate files with expected names (config.yaml, shibboleth.yaml, etc) - No includes - Have C4::Context->config rebuild the 'old structure we already have, and deal with a different new handling on a separate bug (Koha::Config singleton implementation?) - Have a way for C4::Context->config to override any configuration if an env variable exists. e.g. KOHA_DATABASE_NAME should override the relevant entry.

My two cents as well

These are still good ideas. I think we could do them with some very small changes to Koha/Config.pm. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to David Cook from comment #0)

I propose moving /yazgfs/config out of koha-conf.xml and into koha-conf.yml (or even several different *-conf.yml files).

The majority of the /yazgfs child nodes are actually configuration values for zebrasrv (ie Zebra). We should respect /yazgfs as Zebra configuration and leave it alone.

I also propose we move from XML to YAML, as it's more human-friendly and it's easier to parse into the data structure that we need. XML::Simple, which is an obsolete module according to its creator, is currently used to parse koha-conf.xml and it's behaviour can be unpredictable. We should ideally eliminate our usage of it.

Does this report include thinking about how we could secure more sensitive data in our koha-conf file? Like DB password, encryption key, etc. Note e.g. discussion on encryption keys in koha-conf on 34976. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26790 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=31180 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.