GPII-1230: Various matchmaker and lifecycle manager improvements #592
Conversation
|
CI job passed: https://ci.gpii.net/job/universal-tests/717/ |
| if (rootAction === "start" || rootAction === "update") { | ||
| var toSnapshot = gpii.lifecycleManager.removeSettingsBlocks(fluid.copy(solutionRecord)); | ||
|
|
||
| // Disabled the removing of all settingshandlers blocks to ensure that we have them available when restoring the system. |
There was a problem hiding this comment.
Rather than having commenting out code lying around, we should reword the comment to simply reflect the current state - e.g. "Do not remove settingsHandler blocks, since ..."
|
CI job failed: https://ci.gpii.net/job/universal-tests/719/ |
|
CI job passed: https://ci.gpii.net/job/universal-tests/720/ |
|
CI job passed: https://ci.gpii.net/job/universal-tests/721/ |
|
CI job passed: https://ci.gpii.net/job/universal-tests/722/ |
|
CI job passed: https://ci.gpii.net/job/universal-tests/723/ |
| gpii.matchMakerFramework.utils.preProcess = function (initialPayload) { | ||
| var matchMakerInput = fluid.extend({ | ||
| activeContexts: [], // set later in the process | ||
| environmentReporter: {}, // TODO, |
| * if entries for multiple solutions are provided, only the "first" solution entry will be | ||
| * extended. The remaining arguments will be fluid.extended onto the first argument. | ||
| * | ||
| * @currentInstructions {Object} a set of lifecycle instructions keyed by a solution id. For example, |
There was a problem hiding this comment.
Fix up JSDocs here - it should (now) be @param {Object} currentInstructions - A set of etc.
| */ | ||
| gpii.tests.lifecycleManager.extendLifecycleInstructions = function (currentInstructions) { | ||
| var togo = fluid.copy(currentInstructions); | ||
| var solId = Object.keys(currentInstructions)[0]; |
There was a problem hiding this comment.
I think it would well be worth having a check that the length of Object.keys is exactly 1 and if it isn't, exploding
|
|
||
| /* | ||
| * responsible for building the input payload to the matchmaker, via a bunch of helper functions | ||
| * initialPayload consists of: |
There was a problem hiding this comment.
Comment should be clear on whether the input argument is modified since it isn't totally clear from the implementation
There was a problem hiding this comment.
good point! It would depend on the specific matchmaker implementation whether the input is modified - and while I dont think neither the canopy MM nor flat MM touches the inputs right now, I've changed the code to make a copy of it and pass that on - though it goes against all of Kasparnets principles not to leave a time bomb bug like that laying around :)
| // augment payload with inferred common terms | ||
| payload.preferences = gpii.matchMakerFramework.utils.addInferredCommonTerms(payload.preferences, payload.inferredCommonTerms); | ||
| payload.preferences = gpii.matchMakerFramework.utils.addInferredCommonTermsToPreferences(payload.preferences, payload.inferredCommonTerms); | ||
| // gpii.matchMakerFramework.utils.addCapabilitiesInformation(payload); |
There was a problem hiding this comment.
Stray commented line - does this need to stay or go?
There was a problem hiding this comment.
I'll remove it.. This has now been moved to the matchmaker utitilies as part of building the MM input payload
| } | ||
| } | ||
| if (inCanopy) { | ||
| // if solution is already disposed, dont redo it. Previous steps make sure canopy is raised accordingly |
|
CI job failed: https://ci.gpii.net/job/universal-tests/757/ |
|
CI job failed: https://ci.gpii.net/job/universal-tests/777/ |
|
test this please |
|
CI job failed: https://ci.gpii.net/job/universal-tests/796/ |
|
CI job passed: https://ci.gpii.net/job/universal-tests/797/ |
4 participants
This pull request includes Josephs GPII-442 work (the relevant parts of it)
Contains fixes for: GPII-1230, GPII-1235, GPII-2106, GPII-1936, GPII-1750, GPII-1173, GPII-2165, GPII-1930, GPII-2824
Related linux and windows branches: GPII/linux#91, GPII/windows#120
This extends and replaces the old MM pull: #507