add zftp for dataset

[tiantn]

Signed-off-by: tian na tiantn@cn.ibm.com

Proposed changes

Add the zftp extension for dataset to list dataset, list dataset member, edit dataset, upload member, rename dataset, delete dataset and so on.

Release Notes

Milestone:

Changelog:

Types of changes

What types of changes does your code introduce to Zowe Explorer?
Put an x in the boxes that apply

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Updates to Documentation or Tests (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This checklist will be used as reference for both the contributor and the reviewer

• I have read the CONTRIBUTOR GUIDANCE wiki
• PR title follows Conventional Commits Guidelines
• PR Description is included
• gif or screenshot is included if visual changes are made
• yarn workspace vscode-extension-for-zowe vscode:prepublish has been executed
• All checks have passed (DCO, Jenkins and Code Coverage)
• I have added unit test and it is passing
• I have added integration test and it is passing
• There is coverage for the code that I have added
• I have tested it manually and there are no regressions found
• I have added necessary documentation (if appropriate)
• Any PR dependencies have been merged and published (if appropriate)

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

[lauren-li]

Per @tiantn, the following Data Set functionalities should be tested with a zFTP profile:

• List dataset
• List all dataset members
• View dataset content
• Edit dataset
• Edit dataset with confliced content(etag test)
• Create dataset
• Create dataset member
• Show dataset attributes
• Pull from mainframe
• Upload member
• Rename dataset/member
• Delete dataset/member

[jellypuno]

This is super cool! thank you for adding this @tiantn .

Just a few things:

1. For error messages, can you add an indicator that the error happens for zFTP? @lauren-li what do you think? I just feel like that if the user opens an issue with unsupported features then it would be easier for us to identify that the user is using a zFTP profile.

2. Regarding the wrong password in the error above ^^, Can we use the check credentials feature that we have? is there an error code that can identify a wrong password?

3. Additional error handling for functionalities that involves Jobs like Submit Job

4. Minor issue: consistency with error messages. I think it would be a wonderful experience if the error message is the same with Zowe Explorer but this is not required. I think this is just my preference.

For example: Saving a dataset. ZE uses the top one and zFTP uses the bottom one.

[jellypuno]

I encountered errors when testing:

1. Edit dataset with conflicted content
   

2. Copy and Paste. I know this is not supported yet. This can fall under better error message:
   

3. Uploading a member in Favorites. This error appears even though the upload is successful
   

4. Error in renaming datasets in favorites. Same with # 3
   

5. Same happens with delete
   

[IgorCATech]

I'll prepare a release note for Changelog for this feature. If you guys could notify me slightly before you merge this PR, it would be fantastic. This way I can open a PR to merge the Changelog change.

[tiantn]

This is super cool! thank you for adding this @tiantn .

Just a few things:

1. For error messages, can you add an indicator that the error happens for zFTP? @lauren-li what do you think? I just feel like that if the user opens an issue with unsupported features then it would be easier for us to identify that the user is using a zFTP profile.

1. Regarding the wrong password in the error above ^^, Can we use the check credentials feature that we have? is there an error code that can identify a wrong password?

2. Additional error handling for functionalities that involves Jobs like Submit Job

1. Minor issue: consistency with error messages. I think it would be a wonderful experience if the error message is the same with Zowe Explorer but this is not required. I think this is just my preference.

For example: Saving a dataset. ZE uses the top one and zFTP uses the bottom one.

Thank you @jellypuno!

1. I think it will be clear after adding the indicator for the wrong messages. Also helpful to locate the errors.
2. For the wrong password, I tried in my environment and get the following error and different from yours. Don't know if we are in same scenario.
   
3. For the Submit Job, we are now working on the JesAPI, the submit function will be delivered soon.
4. It is a good advice for keeping consistence but I may refine the whole code with the JesAPI PR in next step. Thank you very much!

[tiantn]

I encountered errors when testing:

1. Edit dataset with conflicted content
   ![image](https://user-images.githubusercontent.com/45042840/109943578-a13deb00-7cd5-11eb-94e7-8d8fbcd216d6.png)

2. Copy and Paste. I know this is not supported yet. This can fall under better error message:
   ![image](https://user-images.githubusercontent.com/45042840/109944029-14dff800-7cd6-11eb-9c50-8e89113a6933.png)

3. Uploading a member in Favorites. This error appears even though the upload is successful
   ![image](https://user-images.githubusercontent.com/45042840/109944731-ce3ecd80-7cd6-11eb-9e2d-ab7d64fc5a36.png)

4. Error in renaming datasets in favorites. Same with # 3
   ![image](https://user-images.githubusercontent.com/45042840/109944871-f9292180-7cd6-11eb-8db4-c894f8f937d3.png)

5. Same happens with delete
   ![image](https://user-images.githubusercontent.com/45042840/109944961-1231d280-7cd7-11eb-8bf4-8458114e3bc0.png)

Thank you jellypuno!

1. For the conflict save. Now the error code 412 will be send. And user need do the pull from mainfram manully. Then edit and save again.
2. I am sorry. I can't recreate your errors on other tests. In my environment, they are work well. My colleague will also test it. Hope we can recreate this. Thank you very much for your tests.

[jellypuno]

• For the conflict save. Now the error code 412 will be send. And user need do the pull from mainfram manully. Then edit and save again.

Is it possible to use the compare functionality that we have in Zowe Explorer? If not, A better error message would suffice.

• I am sorry. I can't recreate your errors on other tests. In my environment, they are work well. My colleague will also test it. Hope we can recreate this. Thank you very much for your tests.

To re-create this, you need to add the datasets in Favorites and then perform the functionalities there.

[jellypuno]

2. For the wrong password, I tried in my environment and get the following error and different from yours. Don't know if we are in same scenario.

I have secure credential store installed

3. For the Submit Job, we are now working on the JesAPI, the submit function will be delivered soon.
4. It is a good advice for keeping consistence but I may refine the whole code with the JesAPI PR in next step. Thank you very much!

@lauren-li Are you planning to add the JesAPI PR in 1.13 release? Or will this be an incremental release? Or part of 1.14 release (April 9)

[lauren-li]

@tiantn Thank you for your work on this PR! It is a great start towards getting MVS functionalities with FTP in Zowe Explorer. This helps to address issues #994, #1197 and #1027 (which are all about adding FTP for MVS, but from user and developer perspectives).

Overall, this PR works quite well across the tested functionalities. So far, the issues I ran into have pretty much already all been mentioned by @jellypuno in previous comments. (Thank you Jelly for such thorough testing!)

1. For error messages, can you add an indicator that the error happens for zFTP? @lauren-li what do you think? I just feel like that if the user opens an issue with unsupported features then it would be easier for us to identify that the user is using a zFTP profile

@jellypuno I agree with this. Actually, I think it would be good to include in our conformance criteria that error messages like this should identify the profile type (or extension) where possible. Something like: "Method not yet implemented in FTP".

4. Minor issue: consistency with error messages. I think it would be a wonderful experience if the error message is the same with Zowe Explorer but this is not required. I think this is just my preference.

For example: Saving a dataset. ZE uses the top one and zFTP uses the bottom one.

@jellypuno I agree with this as well! (Maybe also a good conformance criteria recommendation?)

~~
(Moving on to @jellypuno's later comments.)

@tiantn I did not face the issues Jelly mentioned with uploading, renaming, or deleting favorited data sets with FTP. I was able to successfully complete these actions on both unfavorited and favorited PDS and sequential datasets, as well as PDS members. However, for content conflict on save and unsupported functions, it would provide an improved user experience to implement the error handling as Jelly mentioned.

~~

@lauren-li Are you planning to add the JesAPI PR in 1.13 release? Or will this be an incremental release? Or part of 1.14 release (April 9)

@jellypuno That is a good question for @tiantn! :) As our code freeze for the Zowe Explorer 1.13.0 release is planned for this Thursday, I would recommend to focus on polishing these MVS functions for now, and then shoot for 1.14.0 for the JES functions. However, this is really up to @tiantn, as she has done most (if not all) of the work on these generous contributions for FTP.

[tiantn]

@tiantn Thank you for your work on this PR! It is a great start towards getting MVS functionalities with FTP in Zowe Explorer. This helps to address issues #994 and #1027 (which are both about adding FTP for MVS, but from user and developer perspectives).

Overall, this PR works quite well across the tested functionalities. So far, the issues I ran into have pretty much already all been mentioned by @jellypuno in previous comments. (Thank you Jelly for such thorough testing!)

1. For error messages, can you add an indicator that the error happens for zFTP? @lauren-li what do you think? I just feel like that if the user opens an issue with unsupported features then it would be easier for us to identify that the user is using a zFTP profile

@jellypuno I agree with this. Actually, I think it would be good to include in our conformance criteria that error messages like this should identify the profile type (or extension) where possible. Something like: "Method not yet implemented in FTP".

1. Minor issue: consistency with error messages. I think it would be a wonderful experience if the error message is the same with Zowe Explorer but this is not required. I think this is just my preference.

For example: Saving a dataset. ZE uses the top one and zFTP uses the bottom one.

@jellypuno I agree with this as well! (Maybe also a good conformance criteria recommendation?)

~~
(Moving on to @jellypuno's later comments.)

@tiantn I did not face the issues Jelly mentioned with uploading, renaming, or deleting favorited data sets with FTP. I was able to successfully complete these actions on both unfavorited and favorited PDS and sequential datasets, as well as PDS members. However, for content conflict on save and unsupported functions, it would provide an improved user experience to implement the error handling as Jelly mentioned.

~~

@lauren-li Are you planning to add the JesAPI PR in 1.13 release? Or will this be an incremental release? Or part of 1.14 release (April 9)

@jellypuno That is a good question for @tiantn! :) As our code freeze for the Zowe Explorer 1.13.0 release is planned for this Thursday, I would recommend to focus on polishing these MVS functions for now, and then shoot for 1.14.0 for the JES functions. However, this is really up to @tiantn, as she has done most (if not all) of the work on these generous contributions for FTP.

Thank you @lauren-li ! I plan step by step to deliver the functions. We can foucs on the MVS functions first. And then the JES functions. 😊

[jellypuno]

@lauren-li @tiantn ok. we have a plan then 😊

Our next steps would be:

1. @tiantn would push the changes for the error handling before code freeze (latest March 10)
2. @jellypuno would add the error handling in the conformance criteria
3. After the changes are pushed, @lauren-li and @jellypuno and others will review this PR.
4. Merging of this PR should be done before code freeze (latest March 10)
5. Jes APIs for FTP will be added on 1.14 release (April 9)

[std4lqi]

Here are my summary after testing it.

Migrated Dataset: 
* Show Data Set Attribute - Supported
* Add to Favorites - Supported
* Recall Data Set - Unsupported (We need show the explicit error message)

Sequential Dataset:
* Show Data Set Attribute - Supported
* Pull from Mainframe - Supported
* Allocate Like - Unsupported (We need show the explicit error message)
* Copy Data Set - Unsupported (We need show the explicit error message)
* Submit Job - Unsupported (Since job via zFTP is under development)
* Migrate Data Set - Unsupported (We need show the explicit error message)
* Edit Data Set - Supported
* Rename Data Set - Supported
* Delete Data Set - Supported

Partitioned Dataset:
* Show Data Set Attribute - Supported
* Allocate Like - Unsupported (We need show the explicit error message)
* Create New Member - Supported
* Update Member - Supported
* Paste Member - Unsupported (We need show the explicit error message)
* Migrate Data Set - Unsupported (We need show the explicit error message)
* Rename Data Set - Supported
* Delete Data Set - Supported

Partitioned Dataset Member:
* Pull from Mainframe - Supported
* Copy Member - Unsupported (We need show the explicit error message)
* Submit Job - Unsupported (Since job via zFTP is under development)
* Edit Member - Supported
* Rename Member - Supported
* Delete Member - Supported

For the unsupported actions, I can see the error with Recall dataset is not supported is thrown, but hRecallDataSet() in actions.ts doesn't handle it. However, allocateLike() in actions.ts catches the error, and passes it to errorHandling().

From my view, the quick refinement for the error messages of not supported is try/catch MvsApi code and shows message with vscode.window.showErrorMessage(err.message);.

[lauren-li]

@std4lqi Thank you for testing and posting the comprehensive summary! I tried a couple things to get the error handling to work from the PR side, but I think you are right.

@jellypuno Do we want to try and implement the error handling on the Zowe Explorer end for 1.13.0? In addition to hRecallDataSet(), I know hMigrateDataSet() would also benefit from the same refinement. There might be other actions, as well, but I have not had the opportunity to check all of them yet.

[jellypuno]

@lauren-li I am planning to prioritize it next PI 😄 but if you think we should then we can definitely plan it. I am just concerned about capacity. Anyways, I've created an issue about centralized error handling: #388 . Let me know if this is something that you have in mind or maybe you have something else.

[lauren-li]

@lauren-li I am planning to prioritize it next PI 😄 but if you think we should then we can definitely plan it. I am just concerned about capacity. Anyways, I've created an issue about centralized error handling: #388 . Let me know if this is something that you have in mind or maybe you have something else.

@jellypuno Good to hear you are already planning for it, and thank you for creating the issue! I think providing a way for Zowe Explorer extenders to display helpful error messages is important, especially if we are writing specifications for these error messages in our conformance criteria. I was hoping we could do it sometime this PI, but I think you have a valid concern about capacity. (Personally, I am still trying to catch up with my tasks for this sprint.) If we are able to do it earlier, we should. It would definitely be best if we could address Zowe Explorer's implementation before publishing our conformance criteria.

[jellypuno]

From my view, the quick refinement for the error messages of not supported is try/catch MvsApi code and shows message with vscode.window.showErrorMessage(err.message);.

@std4lqi I agree with you. Let's do that for now and then we can enhance it in the future.

[tiantn]

Thank you for your good summary and advices. @jellypuno @lauren-li @std4lqi I added some try/catch in actions.ts for dataset and also updated some messages in ZoweExplorerFtpMvsApi.ts. Could you help to try it again? Thank you very much!

[jellypuno]

@lauren-li @tiantn For some reason I cannot see the error messages that you've added. it seems like when I try to do an action it doesn't go there. Even the Dataset uploaded successfully is not showing. Are you experiencing the same thing or is it just me?

[lauren-li]

@lauren-li @tiantn For some reason I cannot see the error messages that you've added. it seems like when I try to do an action it doesn't go there. Are you experiencing the same thing or is it just me?

@tiantn Thank you for taking the initiative to add the error handling within the Zowe Explorer package! Unfortunately, I am experiencing the same thing as @jellypuno.

To be sure, I tried stopping the watchers and deleting all my node_modules folders before running the yarn command on this PR branch and starting the Zowe Explorer FTP extension in the Extension Development Host. However, I am still seeing the original default VS Code error messages about the error being "likely caused by the extension that contributes [command]". I tried this with the copy/paste and migrate commands.

[tiantn]

@lauren-li @tiantn For some reason I cannot see the error messages that you've added. it seems like when I try to do an action it doesn't go there. Are you experiencing the same thing or is it just me?

@tiantn Thank you for taking the initiative to add the error handling within the Zowe Explorer package! Unfortunately, I am experiencing the same thing as @jellypuno.

To be sure, I tried stopping the watchers and deleting all my node_modules folders before running the yarn command on this PR branch and starting the Zowe Explorer FTP extension in the Extension Development Host. However, I am still seeing the original default VS Code error messages about the error being "likely caused by the extension that contributes [command]". I tried this with the copy/paste and migrate commands.

Thank you @jellypuno @lauren-li . It is very strange. It works well in my environment. I retried it from clone this branch. The following actions I done:

1. git clone --single-branch --branch zftp-dataset git@github.com:tiantn/vscode-extension-for-zowe.git
2. issue "yarn" under /vscode-extension-for-zowe/packages
3. issue "yarn package" under vscode-extension-for-zowe/packages
4. install vscode-extension-for-zowe-1.12.1.vsix and zowe-explorer-ftp-extension-1.12.1.vsix
5. restart vscode.

[lauren-li]

@lauren-li @tiantn For some reason I cannot see the error messages that you've added. it seems like when I try to do an action it doesn't go there. Are you experiencing the same thing or is it just me?

@tiantn Thank you for taking the initiative to add the error handling within the Zowe Explorer package! Unfortunately, I am experiencing the same thing as @jellypuno.
To be sure, I tried stopping the watchers and deleting all my node_modules folders before running the yarn command on this PR branch and starting the Zowe Explorer FTP extension in the Extension Development Host. However, I am still seeing the original default VS Code error messages about the error being "likely caused by the extension that contributes [command]". I tried this with the copy/paste and migrate commands.

Thank you @jellypuno @lauren-li . It is very strange. It works well in my environment. I retried it from clone this branch. The following actions I done:

1. git clone --single-branch --branch zftp-dataset [git@github.com](mailto:git@github.com):tiantn/vscode-extension-for-zowe.git

2. issue "yarn" under /vscode-extension-for-zowe/packages

3. issue "yarn package" under vscode-extension-for-zowe/packages

4. install vscode-extension-for-zowe-1.12.1.vsix and zowe-explorer-ftp-extension-1.12.1.vsix

5. restart vscode.

@tiantn @jellypuno After following @tiantn's method of testing, I am now able to see the new error messages for copy/paste, migrate, and recall. I would have expected this to work from VS Code's Extension Development Host, but maybe something about making simultaneous changes in two separate extensions is giving it trouble. In any case, building/installing the .vsix files for the FTP extension and core Zowe Explorer from this PR branch is working well for me.

Here is the error message I see for copy/paste:

For migrate and recall, I get the new error message, but I also still get the info message that a migrate/recall request was made. I think this can mislead users into thinking maybe the request succeeded. We may want to revise this behavior in a future PR:

I am still doing some additional testing, but so far, I think this PR is in good shape!

[crawr]

I tested the list of supported functionalities and they work well for me. I think this PR brings a lot of value especially as this showcases our extensibility for other connection types.

I saw some inconsistencies in the error messages for the unsupported functions but this can be handled in a subsequent PR.

Thanks for the hard work @tiantn

[lauren-li]

This PR works well for me now. For the Zowe Explorer package, all unit and expected integration tests are passing. In manual testing, the MVS functionalities listed in earlier comments are working with appropriate basic error handling for zFTP profiles, and I did not see regressions with USS.

~
For the next PR to the FTP extension, I have the following suggestions for error handling:

• Implement the Compare functionality for resolving file conflicts when saving. (Or, if that is not done, then address the comment for line 152 of ZoweExplorerFtpMvsApi.ts.)
• Capitalize all instances of "FTP" in error messages.

Thank you @tiantn for this contribution, and for being so responsive to our feedback on it! We greatly appreciate your hard work in introducing the MVS functionality using zFTP profiles to Zowe Explorer.

[lauren-li]

mainfram

Change to: "mainframe"

Because pulling from the mainframe would overwrite any local changes the user was making, I think we should note this in the error message. This can be done in a future PR.