Conversation
…eporting Refactor the complex logic for dealing with merging different types into one place - the ValueList.render method. This removes the replication of the logic in the *item.merge_over methods. As part of the change the errors reported by ValueList.render when merging is not allowed have been changed from standard python TypeErrors to a reclass specific TypeMergeError which has more friendly error reporting giving the parameter name and locations of the error. To accomodate the error reporting change dicts and lists are subclassed in parameters.py to allow a uri tag to placed on a newly created dictionary or list.
The valuelist.render refractor also tightened up several tests which test if an exception was raised. The changes added a test of the message returned by the exception. As the evaluation order of independant reclass parameters is not defined it's possible to get one of several different error message for some tests. python2 uses the same evaluation order on each reclass run but for python3 the order can and does change between reclass runs over the same data. The tests are fixed by allowing any of the valid error messages to pass the test.
|
Added an additional commit to fix the test problems that showed up in the python3 testing. In testing the new TypeMergeError exceptions I wanted to check that not only was the exception raised but also that the correct error message was generated. This is needed to confirm that the correct parameter name and uri's (class files) are part of the error. I forgot I'd also added checks of the error messages returned to some of the other tests. The initial failure of the python3 tests is for one test due to the message property for exceptions going away in python3 and a change in error message. For the rest of the tests the failures are due to the way iteritems works in python2 and python3. For python2 (at least on my laptop) the order of parameter evaluation is the same for the same input data across repeated reclass runs. For python3 this is not true. For example for the infinite recursion error test, running an interpolation on: parameters:
foo: ${bar}
bar: ${foo}can generate an infinite recursion error at either foo or bar depending on which is evaluated first. For python2 (on my laptop) the infinite recursion error was always at bar. For python3 repeated reclass runs generate the infinite recursion error at either foo or bar. The additional commit fixes tests so that either valid error message for the broken tests passes the test. There's also an additional change to an inventory query test to check for two possible outcomes. For similar reasons when a list is returned by an inventory query the list items can be returned in any order. |
|
Seems ok to me, actually my own local model CI that can test with your branch just broke so I will be able to re-test within one week time. I asked one more guy to review. Sure we will merge it. You have time for squash 💃 ) Thanks, the debug output is now super sweet! |
|
Passes my local, complex model -> merging. |
2 participants
This patch fixes a bug with overwriting dictionary parameters. Currently the following:
fails generating a type error exception.
In fixing the bug I ended up refactoring the merging logic that was in the ValueList.render and the *item.merge_over methods into just the ValueList.render method. As well as allowing the original bug to be fixed this simplifies things so that the already complex merging logic is brought together in one place rather than being spread about and duplicated in several methods.
In finishing the refactor the original TypeErrors that were raised when merging fails have been changed to a reclass specific TypeMergeError which uses the more friendly error reporting that gives the parameter name and locations of an error. So that:
becomes: