Support Element getTree convert functions for each TYPE #51

stdweird · 2015-04-03T14:12:11Z

Simplifies generation of configfiles by allowing to convert e.g. all booleans to the strings 'YES' / 'NO', or add quotes around the strings; when the values are retrieved via getTree.
It also allows to e.g. create JSON output corresponding with the original types (e.g. the original JSON profile from the database) when json_typed is enabled.

…upport for additional manipulations based on TYPE

hpcugentbot · 2015-04-03T14:13:30Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/CCM-pr-builder/105/
Test PASSed.

stdweird · 2015-04-03T14:21:54Z

This PR modifies the default internal representation, so anyone using typed_json or xml and e.g. metaconfig with json or yaml module will endup with different (but more "correct") output.
@ned21 @gombasg since MS is using xml, care to comment on the impact of this?

jrha · 2015-04-07T10:38:37Z

What would be the before and after for those using xml?

stdweird · 2015-04-07T11:19:24Z

xml already has typed data, and with this PR long and double are actually long and double (instead of string) when using e.g. metaconfig json and yaml (because of the ::XS bits).

jrha · 2015-04-07T11:54:22Z

What I meant was, when you said this...

anyone using typed_json or xml and e.g. metaconfig with json or yaml module will endup with different (but more "correct") output.

How would the change in output manifest? I assume like this...

Before	After
"dave" : "false"	"dave" : false
"spam" : "42"	"spam" : 42
"eggs" : "tasty"	"eggs" : "tasty"
"tau" : "6.283185"	"tau" : 6.283185

stdweird · 2015-04-07T12:29:11Z

yes. something like that; except for the false example; which would now be 0, which is sometimes is false, sometimes a parse error 😄 "false" is always true in yaml or json. now the pan boolean false can be yaml false or JSON False if you used typed CCM (like xml or typed json).
one of the unittests generates the original JSON profile from the CCM DB as a fun exercise, but we have secrives like cassandra or elasticssearch that use yaml as config file format and do not like quoted strings for integers.
and this is only for YAML::XS and JSON::XS afaik, none of our other components care about the perl internal representation when generating output.

jrha · 2015-04-07T16:22:41Z

😖

Pan false → Written JSON "false" →Read as JSON true

Therefore false == true.

jrha · 2015-04-17T13:03:38Z

src/main/perl/Element.pm

            last SWITCH;
        };

-        # Default clause
+        # Default clause, should never be reached


Famous last words.

If it should never be reached, why not log an error?

because there's no logger here, only throw_error. i don't mind throwing it, but others might disagree
@ned21 toughts?

Throwing an error is generally good but it means that if PAN ever introduces a new data type then data type cannot be used until the node's CCM library is updated to know about it. That's not too unreasonable except that it looks like the current code would handle new types that mapped to Perl scalars just fine anyway, so is the error actually adding any value?

I would be inclined to update the comment instead of throwing an error, e.g. "Default clause: should only be reached if PAN adds a new type which is not explicitly checked above."

Makes sense, updating the comment would be a good idea.

…P4::CCM::Element support

hpcugentbot · 2015-04-19T15:31:44Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/CCM-pr-builder/107/
Test FAILed.

stdweird · 2015-04-19T15:31:45Z

requires quattor/CAF#87

stdweird · 2015-04-19T15:39:49Z

@jrha depends, if you use getTree like ncm-metaconfig does, pan false -> JSON false -> 0 (even without JSON typed support) (but then pan 0 -> json 0 -> possibly "0" (which is true); and also for xml)
anyway, we need a plan to switch to typed json as default, and then merge some more PRs and all is fine again. easy peasy 😄

hpcugentbot · 2015-04-21T11:18:06Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/CCM-pr-builder/108/
Test FAILed.

hpcugentbot · 2015-04-24T17:30:54Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/CCM-pr-builder/109/
Test FAILed.

…type information from within TT files

…methods

…/double/long types

hpcugentbot · 2015-04-26T19:30:31Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/CCM-pr-builder/110/
Test FAILed.

hpcugentbot · 2015-04-26T20:21:43Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/CCM-pr-builder/111/
Test FAILed.

hpcugentbot · 2015-04-26T21:46:32Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/CCM-pr-builder/112/
Test FAILed.

…ions

hpcugentbot · 2015-04-27T06:46:19Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/CCM-pr-builder/113/
Test FAILed.

stdweird · 2015-04-27T06:47:54Z

the new is_long and is_double vmethods only work with typed json and xml; is_boolean works with regular json.

stdweird · 2015-04-27T07:01:39Z

@jrha this PR got a bit out of hand, but it's a huge improvement: correct data representation in XS modules like YAML and JSON; and type checking in TT (no more static lists in the TT files of what is a boolean and what not).
what is currently missing is path support; but i don't know why you'd want that yet.
i'll document the new features in the tutorial.

jrha · 2015-04-27T12:58:26Z

retest this please

hpcugentbot · 2015-04-27T12:59:34Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/CCM-pr-builder/114/
Test PASSed.

jrha · 2015-04-27T13:01:40Z

Ok, I'm happy with this.
Does anyone from MS (@ned21, @gombasg) have any comments?

ned21 · 2015-04-28T21:48:04Z

I agree that in theory this is an improvement, and cannot think of any practical concerns/challenges off hand. Let's give @gombasg chance to reply too. Since this has potentially unknown impact please make sure this is called out in the release notes as a major change so people are aware and can test for problems.

stdweird · 2015-04-29T07:38:42Z

@ned21 this is not only "in theory", we have services that require correct YAML; correct JSON will be useful for ncm-query --format json, but other non-XS/pure-perl output should indeed not be affected.
if you are in doubt, we could enable it via one of the convert methods (and not do anything by default as it was before). it's easy now since there's support for multiple convert methods per type.
then this casting is all moved to usage of CCM::TextRender; and will only affect metaconfig (and even there, only limited to eg yaml or json module).

ned21 · 2015-04-29T14:10:37Z

This seems to offer a benefit to being default, so it would be nice to be enable it as the default--how confident are we that it won't break anything? We can't be afraid to make major changes just because they carry some small risk, but effective communication especially via the release notes can mitigate that risk.

jrha · 2015-04-29T14:37:34Z

Yes this should be the default. I don't see why it should be optional, the current behaviour is effectively a bug, if it causes problems with configuration then those problems should be fixed rather than the current behaviour being preserved.

I will make sure that this change is in big bold text in the release notes. 😄

stdweird · 2015-04-29T15:50:44Z

i'm working on some patches that gives the benefits without changing the defaults. then we don't have to take risks and keep the benefits (the default will shift to usage of CCM::TextRender)

…etTree

…usage of json and yaml modules

hpcugentbot · 2015-04-29T16:54:41Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/CCM-pr-builder/115/
Test PASSed.

stdweird · 2015-04-29T16:55:55Z

@jrha @ned21 the old defaults are back, no loss of functionality

jrha · 2015-04-29T21:52:44Z

Ok, waiting for signoff from @gombasg.

hpcugentbot · 2015-05-01T09:42:51Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/CCM-pr-builder/116/
Test PASSed.

Support Element getTree convert functions for each TYPE

stdweird added 2 commits April 3, 2015 16:07

Force internal perl type to the TYPE and add convert_ anonymous sub s…

a7e1fae

…upport for additional manipulations based on TYPE

Add unittest for (and examples of) the new getTree convert_ options

4761550

stdweird changed the title ~~Support Elment getTree convert functions for each TYPE~~ Support Element getTree convert functions for each TYPE Apr 3, 2015

This was referenced Apr 6, 2015

TextRender: contents from Element instance and getTree convert options quattor/CAF#85

Closed

ncm-metaconfig: pass the contents as Element instance CAF::TextRender quattor/configuration-modules-core#467

Closed

jrha added this to the 15.4 milestone Apr 7, 2015

jrha reviewed Apr 17, 2015
View reviewed changes

Add EDG::WP4::CCM::TextRender, a CAF::TextRender subclass with EDG::W…

c8ab375

…P4::CCM::Element support

stdweird mentioned this pull request Apr 19, 2015

ncm-metaconfig: pass element instance to CCM::TextRender quattor/configuration-modules-core#475

Merged

Clarify comment on default clause in getTree type switch

6c8d00c

Introduce additional variables in TT via CCM namespace

dad225b

stdweird added 4 commits April 26, 2015 20:54

Add CCM::TT::Scalar class to wrap scalar types and provide access to …

5d49f0e

…type information from within TT files

Copy all subdirs (for TT and possible others)

df26263

getTree convert method options are now array references of anonymous …

27c0e45

…methods

CCM::TextRender uses CCM::TT::Scalar instances for the boolena/string…

e549405

…/double/long types

Set 755 permission so the TT subdir has execut bits (and is usable)

5a1ea71

Add the CCM::TextRender Scalar get_type tests

5d0e0ee

CCM::TT::Scalar refine and test the overloading for non-string operat…

1708bc9

…ions

jrha assigned ned21 Apr 27, 2015

stdweird added 3 commits April 29, 2015 18:26

Remove the default conversion for string/long and double in Element g…

20bd123

…etTree

Refactor and add predefined cast convert methods that are implied by …

27df728

…usage of json and yaml modules

Cleanup comments in TT Scalar

b7b45cf

CCM::TextRender add the escape/unescape methods to the CCM VARIABLES

3830674

stdweird mentioned this pull request May 2, 2015

Use CCM::TextRender instead of CAF::TextRender in RegexpTest quattor/maven-tools#53

Merged

ned21 added a commit that referenced this pull request May 3, 2015

Merge pull request #51 from stdweird/support_typed_json

50767a6

Support Element getTree convert functions for each TYPE

ned21 merged commit 50767a6 into quattor:master May 3, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Element getTree convert functions for each TYPE #51

Support Element getTree convert functions for each TYPE #51

stdweird commented Apr 3, 2015

hpcugentbot commented Apr 3, 2015

stdweird commented Apr 3, 2015

jrha commented Apr 7, 2015

stdweird commented Apr 7, 2015

jrha commented Apr 7, 2015

stdweird commented Apr 7, 2015

jrha commented Apr 7, 2015

jrha Apr 17, 2015

stdweird Apr 19, 2015

ned21 Apr 19, 2015

jrha Apr 20, 2015

hpcugentbot commented Apr 19, 2015

stdweird commented Apr 19, 2015

stdweird commented Apr 19, 2015

hpcugentbot commented Apr 21, 2015

hpcugentbot commented Apr 24, 2015

hpcugentbot commented Apr 26, 2015

hpcugentbot commented Apr 26, 2015

hpcugentbot commented Apr 26, 2015

hpcugentbot commented Apr 27, 2015

stdweird commented Apr 27, 2015

stdweird commented Apr 27, 2015

jrha commented Apr 27, 2015

hpcugentbot commented Apr 27, 2015

jrha commented Apr 27, 2015

ned21 commented Apr 28, 2015

stdweird commented Apr 29, 2015

ned21 commented Apr 29, 2015

jrha commented Apr 29, 2015

stdweird commented Apr 29, 2015

hpcugentbot commented Apr 29, 2015

stdweird commented Apr 29, 2015

jrha commented Apr 29, 2015

hpcugentbot commented May 1, 2015

Support Element getTree convert functions for each TYPE #51

Support Element getTree convert functions for each TYPE #51

Conversation

stdweird commented Apr 3, 2015

hpcugentbot commented Apr 3, 2015

stdweird commented Apr 3, 2015

jrha commented Apr 7, 2015

stdweird commented Apr 7, 2015

jrha commented Apr 7, 2015

stdweird commented Apr 7, 2015

jrha commented Apr 7, 2015

jrha Apr 17, 2015

Choose a reason for hiding this comment

stdweird Apr 19, 2015

Choose a reason for hiding this comment

ned21 Apr 19, 2015

Choose a reason for hiding this comment

jrha Apr 20, 2015

Choose a reason for hiding this comment

hpcugentbot commented Apr 19, 2015

stdweird commented Apr 19, 2015

stdweird commented Apr 19, 2015

hpcugentbot commented Apr 21, 2015

hpcugentbot commented Apr 24, 2015

hpcugentbot commented Apr 26, 2015

hpcugentbot commented Apr 26, 2015

hpcugentbot commented Apr 26, 2015

hpcugentbot commented Apr 27, 2015

stdweird commented Apr 27, 2015

stdweird commented Apr 27, 2015

jrha commented Apr 27, 2015

hpcugentbot commented Apr 27, 2015

jrha commented Apr 27, 2015

ned21 commented Apr 28, 2015

stdweird commented Apr 29, 2015

ned21 commented Apr 29, 2015

jrha commented Apr 29, 2015

stdweird commented Apr 29, 2015

hpcugentbot commented Apr 29, 2015

stdweird commented Apr 29, 2015

jrha commented Apr 29, 2015

hpcugentbot commented May 1, 2015