ncm-nss: replace LC::Check with FileWriter #810
Conversation
| @@ -0,0 +1,15 @@ | |||
| object template simple; | |||
|
|
|||
| #function pkg_repl = { null; }; | |||
There was a problem hiding this comment.
It would be nice to test the schema too by including config.pan but I can't remember why this didn't work now - I wrote the code too long ago. :( What's recommended best practice here?
There was a problem hiding this comment.
remove these comments to start with 😄
and change https://github.com/quattor/configuration-modules-core/blob/master/ncm-nss/src/main/pan/components/nss/schema.pan#L11 in https://github.com/quattor/configuration-modules-core/blob/master/ncm-autofs/src/main/pan/components/autofs/schema.pan#L7
(and include any other types needed in the schema, but from quick glance, none are required)
There was a problem hiding this comment.
user-defined validation failed
element path: '/software/components/nss/version'
element value: 16.2.1-SNAPSHOT
because the component type only allows digits in this field.
There was a problem hiding this comment.
use 'version' ?= '${no-snapshot-version}';
| function pkg_repl = { null; }; | ||
| include 'components/nss/config'; | ||
| "/software/components/nss/dependencies/pre" = null; | ||
| # a version of 16.2.1-SNAPSHOT fails validation so unset |
Use FileWriter instead of LC::Check and add a simple unit test of the behaviour. Also minor clean up to the logging to use info instead of just log(), and do not output a confusing statement in --noaction mode. Minor whitespace clean up.
| if ($result) { | ||
| $self->log("updated /etc/nsswitch.conf"); | ||
| $self->info("updated /etc/nsswitch.conf") unless $NoAction; |
There was a problem hiding this comment.
this looks like some idiom to add to CAF, but also related to quattor/CAF#100.
in any case, most components do not make this distinction (and it should be handeled on e.g. CAF level).
anyway, what is interesting is that the tests pass without any definition of $NoAction (i expected use strict or use warning to at least make some noise)
There was a problem hiding this comment.
wrt NoAction, in the old components code, we extend the NCM::Component namespace by diretc manipulation of ISA, and the $NoAction variable is the one set here: https://github.com/quattor/ncm-ncd/blob/master/src/main/perl/Component.pm#L15
if you would switch to the use parent qw(NCM::Component), this would fail, and you would have to use $CAF::Object::NoAction. as the NoAction is read most of the time, this is ok.
|
Anything to be done here to get this into 16.8? |
| "/software/components/${project.artifactId}/active" ?= true ; | ||
| "/software/components/${project.artifactId}/dispatch" ?= false ; | ||
| "/software/components/${project.artifactId}/version" = "${project.version}"; | ||
| "/software/components/${project.artifactId}/active" ?= true ; |
There was a problem hiding this comment.
copy paste the code in the comment referenced above, e.g. to use prefix here
|
small remark wrt to the pan template cleanup. can you also open an issue in |
|
config.pan updated and quattor/CAF#185 opened. |
|
@ned21 thx |
Use FileWriter instead of LC::Check and add a simple unit test of the
behaviour.