Dancer icon indicating copy to clipboard operation
Dancer copied to clipboard

Use of YAML.pm to read config is hard-coded (so YAML::XS is not used even when specified)

Open 1nickt opened this issue 7 years ago • 2 comments

Hello,

<TL;DR> The docs state you can specify YAML::XS for reading config, but Dancer::Config hard-codes use of YAML. </TL;DR>

I have been trying to use YAML::XS instead of YAML for reading the Dancer config (mostly because YAML.pm doesn't support in-line comments as per the YAML spec).

The docs for Dancer::Config state:

By default, the module YAML will be used to parse the configuration files. If desired, it is possible to use YAML::XS instead by changing the YAML engine configuration in the application code:

config->{engines}{YAML}{module} = 'YAML::XS';

See Dancer::Serializer::YAML for more details.

The docs for Dancer::Serializer::YAMLstate:

By default, the module YAML will be used to serialize/deserialize data and the application configuration files. This can be changed via the configuration:

engines:
    YAML:
        module: YAML::XS

Note that if you want all configuration files to be read using YAML::XS, that configuration has to be set via application code:

config->{engines}{YAML}{module} = 'YAML::XS';

However, the code in Dancer::Config reads (starting around l.227) :

sub load_settings_from_yaml {
    my ($file) = @_;

    my $config = eval { YAML::LoadFile($file) }
        or confess "Unable to parse the configuration file: $file: $@";

... which seems to hard-code use of YAML.

Also, the code for Dancer::Serializer::YAML reads (starting around l. 43):

sub serialize {
    my ($self, $entity) = @_;
    YAML::Dump($entity);
}

sub deserialize {
    my ($self, $content) = @_;
    YAML::Load($content);
}

... which also seems to hard-code use of YAML.

I would be happy to submit a patch to fix this, if there is any interest from the maintainers in applying the fix.

1nickt avatar Oct 11 '16 14:10 1nickt

Hmm. Given the feature-freeze in D1, I'm mostly inclined to say "we'll update the docs to remove the suggestion that you can override the use of YAML" - but I realise that doesn't help you with your requirement.

I'll give it a little thought - if it can be made possible to use YAML::XS by setting that setting before the config is read - but I have a vague recollection that calling Dancer->config, even when using it as a mutator?

As mentioned on IRC, looks like PR #1072 already implemented it, but was closed by f0585d5b - so at least most of the work is already done it would seem.

bigpresh avatar Oct 11 '16 22:10 bigpresh

Hi @bigpresh, PR raised here :-)

I didn't add support for YAML::XS in session management, just reading config and serialization. So now I can have in-line comments in my YAML, whee!

All tests passing and live app round-tripping YAML content also tested.

If it's not pushing it and you could see your way clear to having a look at this earlier PR as well, that'd be brilliant.

Thanks!

1nickt avatar Oct 12 '16 21:10 1nickt