Enhance Installation Stage to Run 'zwe init mvs' #83
Conversation
…type. Added method to run zwe init mvs Signed-off-by: Timothy Gerstel <tim.gerstel@gmail.com>
Signed-off-by: Timothy Gerstel <tim.gerstel@gmail.com>
src/actions/InstallationHandler.ts
Outdated
| } | ||
|
|
||
| console.log("running zwe init mvs..."); | ||
| const initMvs = await this.initMVS(connectionArgs, installationArgs.installationDir); |
There was a problem hiding this comment.
it looks to me like init mvs is run regardless of the status of install.
i think this file needs to be reorged a little bit to have a step only continue if the previous step succeeded.
because if you didnt upload, you cant unpax. if you didnt unpax, you cant install. if you didnt install, you cant init.
Signed-off-by: Timothy Gerstel <tim.gerstel@gmail.com>
src/actions/InstallationHandler.ts
Outdated
| initMvs = await this.initMVS(connectionArgs, installationArgs.installationDir); | ||
| ProgressStore.set('installation.initMVS', initMvs.status); | ||
| } else { | ||
| initMvs = {status: true} |
There was a problem hiding this comment.
if installMvs = false, initMvs = true?
unconditionally?
i can't think of why this wouldnt be false.
Signed-off-by: Timothy Gerstel <tim.gerstel@gmail.com>
…nitMvs Signed-off-by: Timothy Gerstel <tim.gerstel@gmail.com>
|
|
||
| return {status: download.status && uploadYaml.status && upload.status && unpax.status && install.status, details: ''}; | ||
| return {status: download.status && uploadYaml.status && upload.status && unpax.status && installation.status && initMvs.status, details: ''}; | ||
| } catch (error) { | ||
| return {status: false, details: error.message}; |
There was a problem hiding this comment.
This code does not properly present errors to users.
We have a try { } catch { } here, but not all failures go to the catch { } block.
If init mvs fails, there is no exception but the return would say {status: false, details: ''}.
There would be no way to differentiate that from a zwe install or extract step failure.
You should set details to what actually failed.
In fact, initMVS() is returning a details, so it could be formatted.
There was a problem hiding this comment.
This return statement existed prior to this PR, I'd be happy to fix it but I think it should be a separate ticket to enhance the error detailing of this stage @1000TurquoisePogs
There was a problem hiding this comment.
@timgerstel @1000TurquoisePogs don't forget, if there are new errors in Zen the user needs to see, use this:
https://github.com/zowe/zen/pull/45/files#diff-6d106a102cd6a61731f4cb784b463952dd7e6c02a2f1ba3df3e4602e9a4e7e45R222
Signed-off-by: Timothy Gerstel <tim.gerstel@gmail.com>
Signed-off-by: Timothy Gerstel <tim.gerstel@gmail.com>
There was a problem hiding this comment.
It is an improvement so I will merge it but there still are situations that need to be caught:
If you run it twice, because "--allow-overwrite" is not used, technically init does nothing the second time.
It prints out warnings and then says success.
Those warnings should be propagated to the UI, which can be done in a follow-up PR.
Proposed changes
This PR addresses Issue: [Link to Github issue within https://github.com/zowe/zen/issues if any]
This PR depends upon the following PRs:
Type of change
Please delete options that are not relevant.
PR Checklist
Please delete options that are not relevant.
Testing
Further comments