-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
zwe install
implementation details
#4044
Comments
I think the 2 CLI.js approach is valid because we dont want to dilute the principles of configmgr schema validation, but we also don't want to lose features by forcing things to use a YAML when not required. So, yeah, the command variant that uses the YAML should do configmgr validation, and the one that doesnt could be generic quickJS code. |
I would like to take a closer look, but at a glance, I prefer option (1) or (2). The split CLI.js approach is valid and makes sense, but I'm a little worried about maintenance over time if we're essentially duplicating the bulk of the implementation code in two places, one with configmgr validation and one without. DRY, code drift, and all that. For option (3), my question is: is the benefit to developers worth the additional code maintenance? Or is there another way to make this developer friendly - like |
SplitThe split (so far) was done in the minimal way:
The CLIs together are approx 20 lines, the majority of the code is common in the Tmp configWe can create in EditPOC is working, we get the benefit of schema validation of DSN, but also this error, if DSN is not valid:
This might be confusing for users, if you don't specify yaml, but you have an error there. |
If configmgr is mandatory (which it should be IMHO), then implicit uses of it for defaults might need more user-friendly failure messages. Some users don't know that they are using YAML behind the scenes and that it is getting validated by JSON Schema |
Could we work around this edge case of zwe install encountering schema validation errors by validating the "--ds-prefix" input in the shell script that runs directly prior to configmgr being invoked? |
Yes, of course. If we decide to use #4057, we can test |
As moving to JCL templates,
zwe install
should be rewritten too. All we need for installation iszowe.runtimeDirectory
andzowe.setup.dataset.prefix
.Current implementation supports 2 ways:
zwe install --config /path/to/zowe.yaml
zwe install --ds-prefix ZOWE.V3R0M0
zowe.yaml
, this is handy for quick installation, developers...There is a problem with
import
, if we use the same approach as for other commands:At some place we need to use
import * as config
orimport * as configmgr
: this will cause the code will require config. But we need also code path for--ds-prefix
, which means without config.Solutions:
--ds-prefix
- I would like to keep it, I am using it 😄Update
Problems:
--ds-prefix
user does not have an option to customize JCL header - unless we will add another parameterZWEINSTL
creates datasets and copies files (SHell in BPXBATCH)--allow-overwrite
is used, we need to skip allocating or delete datasets or have 2 JCL, one with allocate+shell and the second with shell only or something betterThe text was updated successfully, but these errors were encountered: