action #59148

Support YAML merge keys in Job Group Editor

Added by tinita 4 months ago. Updated about 16 hours ago.

Status:FeedbackStart date:06/11/2019
Priority:NormalDue date:
Assignee:tinita% Done:

0%

Category:Feature requestsEstimated time:20.00 hours
Target version:Current Sprint
Difficulty:
Duration:

Description

The Job group editor is using YAML::XS, which does not support YAML merge keys, which allow you to merge several mappings into one:
https://yaml.org/type/merge.html

E.g.

---
defaults: &defaults
  a: 1
  b: 2
settings:
  <<: *defaults
  b: 22
  c: 3

There is currently only one module in perl supporting that: YAML::PP https://metacpan.org/pod/YAML::PP.
It allows to use libyaml as a parser backend via YAML::PP::LibYAML https://metacpan.org/pod/YAML::PP::LibYAML, so we can rely on the battle tested parsing of libyaml while using the features of the pure perl loader.

The only disadvantage is that YAML::PP is still pretty new and not yet "battle tested" like YAML::XS.

The advantage is, that YAML::PP adheres to the YAML 1.2 spec (regarding the schema, e.g. TRUE, True, true are booleans), while YAML::XS did only implement a small subset of YAML 1.1 types.
So it will be compatible to other YAML 1.2 processors.

The change in the code itself would be easy:

Instead of

my $data = YAML::XS::Load(...);

-->

my $yp = YAML::PP->new( schema => [qw/ + Merge /] );
my $data = $yp->load_string(...);

Checklist

  • Add YAML::PP (and poosibly YAML::PP::LibYAML) deps and submit them to Factory/Leap
  • Change code to use the new module
  • Add tests
  • Possibly check all existing templates if they can be parsed with the new module
  • Add documentation on how to use merge keys
  • Add a gerneral description for YAML 1.2 (e.g. which strings are special)

Related issues

Related to openQA Project - action #62603: Anchor/alias for hash map in job template settings in Job... Rejected 23/01/2020

History

#1 Updated by coolo 4 months ago

  • Target version set to future

As long as we have enough test coverage, an unstable api shouldn't hurt - mojolicious suprises us from time to time as well :)

But for now I don't want to invest further into complex yaml features.

#2 Updated by tinita about 1 month ago

  • Related to action #62603: Anchor/alias for hash map in job template settings in Job Groups YAML produces error added

#3 Updated by tinita about 1 month ago

  • Estimated time set to 20.00

#4 Updated by tinita about 1 month ago

  • Checklist set to [ ] Add YAML::PP (and poosibly YAML::PP::LibYAML) deps and submit them to Factory/Leap, [ ] Change code to use the new module, [ ] Add tests
  • Description updated (diff)

#5 Updated by tinita about 1 month ago

  • Checklist changed from [ ] Add YAML::PP (and poosibly YAML::PP::LibYAML) deps and submit them to Factory/Leap, [ ] Change code to use the new module, [ ] Add tests to [ ] Add YAML::PP (and poosibly YAML::PP::LibYAML) deps and submit them to Factory/Leap, [ ] Change code to use the new module, [ ] Add tests, [ ] Possibly check all existing templates if they can be parsed with the new module

#6 Updated by tinita 26 days ago

  • Checklist set to [x] Possibly check all existing templates if they can be parsed with the new module

#7 Updated by tinita 26 days ago

Checked templates with:

% lwp-request -m GET https://openqa.opensuse.org/api/v1/job_templates_scheduling >templates.yaml
% perl -wE'
  use YAML::PP;
  use YAML::PP::LibYAML;
  my $file = "templates.yaml";
  my $data = YAML::PP::LoadFile($file);
  for my $key (sort keys %$data) {
    say $key;
    my $yaml = $data->{ $key };
    my $template = YAML::PP::Load($yaml);
    $template = YAML::PP::LibYAML::Load($yaml);
  }'
# same with openqa.suse.de
% lwp-request -m GET https://openqa.suse.de/api/v1/job_templates_scheduling >templates.yaml
% ...

#8 Updated by tinita 7 days ago

  • Description updated (diff)

#9 Updated by coolgw 7 days ago

I like this feature, i think we even can start discard test suite if this support.
We just need create a template within yaml, and many other case can use template with some specific setting.
Hope this can be supported in near future.

#10 Updated by tinita 7 days ago

  • Status changed from New to In Progress
  • Assignee set to tinita
  • Target version changed from future to Current Sprint

#11 Updated by tinita 7 days ago

YAML::PP and YAML::LibYAML::API are already in openQA:devel.
(I found a bug in YAML::LibYAML::API though and released version 0.008 today).

I submitted YAML::PP::LibYAML today.

I changed the code to use YAML::PP. As soon as the *LibYAML* packages are in the repo, I can replace it by YAML::PP::LibYAML.

I also added a test for merge keys.

#12 Updated by tinita 7 days ago

  • Checklist set to [x] Change code to use the new module

#13 Updated by tinita 7 days ago

  • Checklist set to [x] Add tests

#14 Updated by tinita 7 days ago

  • Checklist changed from [ ] Add YAML::PP (and poosibly YAML::PP::LibYAML) deps and submit them to Factory/Leap, [x] Change code to use the new module, [x] Add tests, [x] Possibly check all existing templates if they can be parsed with the new module to [ ] Add YAML::PP (and poosibly YAML::PP::LibYAML) deps and submit them to Factory/Leap, [x] Change code to use the new module, [x] Add tests, [x] Possibly check all existing templates if they can be parsed with the new module, [ ] Add documentation on how to use merge keys, [ ] Add a gerneral description for YAML 1.2 (e.g. which strings are special)

#15 Updated by tinita 7 days ago

  • Checklist set to [x] Add documentation on how to use merge keys

#16 Updated by tinita 7 days ago

  • Checklist set to [x] Add a gerneral description for YAML 1.2 (e.g. which strings are special)

#17 Updated by tinita 7 days ago

Created a draft PR: https://github.com/os-autoinst/openQA/pull/2751

Now waiting for the libyaml packages in OBS

#18 Updated by tinita 6 days ago

As stated in the PR, I think we won't see a performance impact by using YAML::PP, since the files are only loaded in the Jobtemplate editor.

#19 Updated by tinita 6 days ago

  • Checklist changed from [ ] Add YAML::PP (and poosibly YAML::PP::LibYAML) deps and submit them to Factory/Leap, [x] Change code to use the new module, [x] Add tests, [x] Possibly check all existing templates if they can be parsed with the new module, [x] Add documentation on how to use merge keys, [x] Add a gerneral description for YAML 1.2 (e.g. which strings are special) to [x] Add YAML::PP (and poosibly YAML::PP::LibYAML) deps and submit them to Factory/Leap, [x] Change code to use the new module, [x] Add tests, [x] Possibly check all existing templates if they can be parsed with the new module, [x] Add documentation on how to use merge keys, [x] Add a gerneral description for YAML 1.2 (e.g. which strings are special)
  • Status changed from In Progress to Feedback

#20 Updated by tinita 4 days ago

PR was merged!

#21 Updated by tinita 4 days ago

Deployed on o3 and osd

Also available in: Atom PDF