Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Describe YAML job templates editor as default with migration script from old #2524

Merged
merged 1 commit into from
Nov 19, 2019

Conversation

okurz
Copy link
Member

@okurz okurz commented Nov 19, 2019

As the old job templates editor is deprecated we should more prominently
describe the new YAML job templates editor but also describe better how
to migrate including a scripted way to batch-process.

Related progress issue: https://progress.opensuse.org/issues/60020#change-258569

…ript from old

As the old job templates editor is deprecated we should more prominently
describe the new YAML job templates editor but also describe better how
to migrate including a scripted way to batch-process.

Related progress issue: https://progress.opensuse.org/issues/60020#change-258569
@codecov
Copy link

codecov bot commented Nov 19, 2019

Codecov Report

Merging #2524 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2524      +/-   ##
==========================================
- Coverage   87.05%   87.03%   -0.02%     
==========================================
  Files         172      172              
  Lines       11008    11008              
==========================================
- Hits         9583     9581       -2     
- Misses       1425     1427       +2
Impacted Files Coverage Δ
lib/OpenQA/WebAPI/Command/gru/run.pm 92.3% <0%> (-7.7%) ⬇️
...ib/OpenQA/WebAPI/Plugin/ObsRsync/Controller/Gru.pm 92.5% <0%> (-5%) ⬇️
lib/OpenQA/Worker/Engines/isotovideo.pm 93.98% <0%> (+0.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 984ebfb...eb22e32. Read the comment docs.

@okurz okurz merged commit d891c85 into os-autoinst:master Nov 19, 2019
@okurz okurz deleted the feature/yaml_migrate branch November 19, 2019 18:32
@AdamWill
Copy link
Contributor

so, I can't quite grok this: in Fedora we barely use the web UI for editing this stuff. Our 'canonical' source of info is our templates file - which is still in perl format for now - and we reload the templates from file every time we change anything.

Do I need to worry about this at all? Is our old-school template file going to stop working at some point unless I change it somehow? If so, how?

@okurz
Copy link
Member Author

okurz commented Nov 22, 2019

I think you are following a good practice but, yes, I guess eventually the old format will probably stop to work when we continue the route of using YAML format to describe job templates and propably eventually also the content of other tables, e.g. test suites.

@coolo, can you further comment on this?

@Martchus
Copy link
Contributor

@AdamWill As far as I know we do quite the opposite. Editing happens mostly in the web UI and the load/dump scripts are only used to transfer the setup from one instance to another. I'm not sure whether we use the scripts (and their data format) for anything persistent like backups. Nevertheless I would say we should try not to change the data format of the scripts then.

Note that we so far also dump/load the raw YAML for job templates with those scripts (to preserve the exact syntax). Obviously editing the YAML embedded in a JSON document (as a string) doesn't make too much sense. I guess we even still save the actual job template entries at the same time. Not sure how an inconsistency would affect loading such a file. @kalikiana Maybe you know more/better?

@AdamWill
Copy link
Contributor

"As far as I know we do quite the opposite"

Yeah, that's right, we've always had the opposite workflow here. Not sure how it started, I think when Richard first did his Fedora openQA prototype it included loading the initial config from a templates file and we just kinda picked that up as the workflow (edit templates file, load it). Also it's a requirement in Fedora infra that all deployments be 100% scripted, so loading in the test config from the templates file is part of those deployment scripts.

As I mentioned, our templates files aren't even JSON at present - they're basically perl. This was the required format when we started, support for JSON templates was only added subsequently. If you look at script/load_templates, its loading code is basically this:

try {
    $info = Cpanel::JSON::XS->new->relaxed->decode($datafile);
    dd $info;
}
catch {
    $info = eval $datafile;
    if (my $error = $@) {
        die "Error in data file: $error\n";
    }
};

i.e. try and read it as JSON, if that doesn't work, just eval it - that's the bit that works for files in the old format.

So I guess the scenarios I can see happening would be, some change gets made to how job templates work that requires the YAML format and then our templates won't load any more (or break openQA or something)...or we do go into the UI editor for some reason, make some change, and after that loading our templates from the 'old' format breaks things somehow.

I don't know the details here, but just as an idea, would it make things cleaner if we just made YAML the official format for templates, and deprecated both perl-y and JSON templates, with some kinda conversion tool? That way we could convert our templates to YAML and be more confident in future...

@okurz
Copy link
Member Author

okurz commented Nov 22, 2019

I don't know the details here, but just as an idea, would it make things cleaner if we just made YAML the official format for templates, and deprecated both perl-y and JSON templates, with some kinda conversion tool? That way we could convert our templates to YAML and be more confident in future...

That is basically also my idea, yes. Converting from perl hashes to JSON to YAML is trivial though. A challenge can only arise when you map levels differently and such. Let's hope everybody remembers to keep the migration paths open :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants