[FTP ext] Close connection #1324

tiantn · 2021-05-20T05:53:45Z

Proposed changes

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

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...

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

lauren-li · 2021-05-25T13:08:15Z

Summary of changes in this PR:
This PR addresses issue #1196 (FTP Extension - Numerous FTP tasks started on the host) by doing the following:

Add the releaseConnection function that closes a connection.
Wrap each FTP operation in a try/finally statement.
CallrelaseConnection in the finally block to close the connection at the end of each FTP operation.

@tiantn @std4lqi Please correct me if any of the above is wrong or incomplete. Thanks!

tiantn · 2021-05-26T01:47:43Z

Summary of changes in this PR:
This PR addresses issue #1196 (FTP Extension - Numerous FTP tasks started on the host) by doing the following:
1. Add the `releaseConnection` function that closes a connection.

2. Wrap each FTP operation in a `try`/`finally` statement.

3. Call`relaseConnection` in the `finally` block to close the connection at the end of each FTP operation.
@tiantn @std4lqi Please correct me if any of the above is wrong or incomplete. Thanks!

Hi @lauren-li, I think you are correct. Thank you!

Signed-off-by: Lauren Li <45975633+lauren-li@users.noreply.github.com>

lauren-li · 2021-05-28T13:23:21Z

@tiantn The code on this branch looks good to me so far. I've made a PR into your close-connection branch to update it with the latest changes in Zowe Explorer's master branch: https://github.com/tiantn/vscode-extension-for-zowe/pull/1

Could you please use this to bring your PR up-to-date with master branch? (Or, if it's easier for you, feel free to directly update your close-connection branch instead.) Apologies for the trouble, and thank you for all your work on this!

Close connection - update with master

tiantn · 2021-05-31T02:37:44Z

@tiantn The code on this branch looks good to me so far. I've made a PR into your close-connection branch to update it with the latest changes in Zowe Explorer's master branch: tiantn#1

Could you please use this to bring your PR up-to-date with master branch? (Or, if it's easier for you, feel free to directly update your close-connection branch instead.) Apologies for the trouble, and thank you for all your work on this!

Thank you @lauren-li ! Thank you for helping resolving the conflicts. 😊

katelynienaber · 2021-06-04T11:02:42Z

@lauren-li @tiantn Hello, sorry for lateness. I wanted to get some help from @VitGottwald with testing this.

It seems to me that the issue described in that bug is still occurring. I tested it using the setup described in the related issue (new FTP profile, used in USS tree to open/create/delete folders & files) and in z/OSMF I still get a large list of new jobs (one for each action performed) which do not stop until I close VSCode.

lauren-li · 2021-06-04T20:25:27Z

It seems to me that the issue described in that bug is still occurring.

@katelynienaber Just to double-check, are you testing this from "Run Zowe Explorer FTP VS Code Extension"?

(Alternatively, you can run yarn package to build the vsix file for the FTP extension, install it in VS Code, and test that way.)

lauren-li

This PR seems to fix issue #1196 for me. I see at most 1 FTP task resulting from using the Zowe Explorer FTP extension now, even if I'm opening and closing multiple files/folders and performing various searches.

However, I did encounter an ESLint error when trying to build the vsix file with yarn package. (See comment below for details.) I think this should be addressed within this PR.

Overall, I think this PR is almost ready to go. Thanks @tiantn for your hard work on this PR, and @std4lqi for your guidance!

lauren-li · 2021-06-04T20:54:46Z

packages/zowe-explorer-ftp-extension/src/ZoweExplorerFtpJesApi.ts

+                const options = {
+                    owner: owner,
+                };
+                const response = (await JobUtils.listJobs(connection, prefix, options)) as IJob[];


When running yarn package, I received the following ESLint error:

To resolve this error, I had to delete as IJob[] from the end of line 34.

Yes, I think as IJob[] is unnecessary now. JobUtils.listJobs returns Promise<IJob[]>. Thanks!

public static async listJobs(connection: any, prefix: string, option?: IListJobOption): Promise<IJob[]>

Thank you @std4lqi for confirming this. I've pushed a commit removing as IJob[] to the PR branch. (@tiantn I hope this is ok with you! I just figured it would be quicker to do a direct push since this was a small, low-risk change.)

tiantn · 2021-06-07T02:37:02Z

It seems to me that the issue described in that bug is still occurring.

@katelynienaber Just to double-check, are you testing this from "Run Zowe Explorer FTP VS Code Extension"?

(Alternatively, you can run yarn package to build the vsix file for the FTP extension, install it in VS Code, and test that way.)

@lauren-li @tiantn Hello, sorry for lateness. I wanted to get some help from @VitGottwald with testing this.

It seems to me that the issue described in that bug is still occurring. I tested it using the setup described in the related issue (new FTP profile, used in USS tree to open/create/delete folders & files) and in z/OSMF I still get a large list of new jobs (one for each action performed) which do not stop until I close VSCode.

Hi @katelynienaber , as @lauren-li suggested, you can build the vsix file for the FTP extension and install it to try again. Thank you!

katelynienaber

Yes, I think I used the wrong extension >.>

This is working for me, all started tasks disappear in the MF activity log once they are completed.

Signed-off-by: Lauren Li <45975633+lauren-li@users.noreply.github.com>

…-for-zowe into close-connection

lauren-li

This PR looks good to me now! All unit and expected integration tests are passing, and manual testing shows that the tasks generated through using the FTP extension go away once they are complete.

Thanks @tiantn for putting this PR together, and thanks @std4lqi for helping out!

tiantn · 2021-06-08T02:10:51Z

Thanks @lauren-li @std4lqi 😊

katelynienaber

LGTM :)

tiantn added 4 commits May 17, 2021 16:37

close-connection

b64409d

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

using releaseConnection and add check connection

57e213f

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

eslint adjust

5180788

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

update releaseConection

ab6c1f6

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

jellypuno changed the title ~~Close connection~~ [FTP ext] Close connection May 20, 2021

mmoore96 mentioned this pull request May 25, 2021

zOS FTP Multiple UNIX processes zowe/zowe-cli-ftp-plugin#78

Closed

crawr added this to the 1.16.0 Release milestone May 27, 2021

Merge in master and resolve conflicts

a3b0ca3

Signed-off-by: Lauren Li <45975633+lauren-li@users.noreply.github.com>

Merge pull request #1 from zowe/close-conn-update

8f3650f

Close connection - update with master

crawr requested review from jellypuno, lauren-li and katelynienaber June 1, 2021 14:40

Merge branch 'master' into close-connection

2ea8b16

lauren-li reviewed Jun 4, 2021

View reviewed changes

katelynienaber previously approved these changes Jun 7, 2021

View reviewed changes

jellypuno removed their request for review June 7, 2021 10:05

katelynienaber and others added 3 commits June 7, 2021 12:06

Merge branch 'master' into close-connection

17efef5

Remove as IJob

47e3bcf

Signed-off-by: Lauren Li <45975633+lauren-li@users.noreply.github.com>

Merge branch 'close-connection' of github.com:tiantn/vscode-extension…

ce68885

…-for-zowe into close-connection

lauren-li dismissed katelynienaber’s stale review via ce68885 June 7, 2021 13:07

lauren-li approved these changes Jun 8, 2021

View reviewed changes

katelynienaber approved these changes Jun 8, 2021

View reviewed changes

lauren-li merged commit 7cf5454 into zowe:master Jun 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FTP ext] Close connection #1324

[FTP ext] Close connection #1324

tiantn commented May 20, 2021

lauren-li commented May 25, 2021

tiantn commented May 26, 2021

lauren-li commented May 28, 2021

tiantn commented May 31, 2021

katelynienaber commented Jun 4, 2021 •

edited

Loading

lauren-li commented Jun 4, 2021 •

edited

Loading

lauren-li left a comment

lauren-li Jun 4, 2021

std4lqi Jun 7, 2021

lauren-li Jun 8, 2021

tiantn commented Jun 7, 2021

katelynienaber left a comment

lauren-li left a comment

tiantn commented Jun 8, 2021

katelynienaber left a comment

[FTP ext] Close connection #1324

[FTP ext] Close connection #1324

Conversation

tiantn commented May 20, 2021

Proposed changes

Release Notes

Types of changes

Checklist

Further comments

lauren-li commented May 25, 2021

tiantn commented May 26, 2021

lauren-li commented May 28, 2021

tiantn commented May 31, 2021

katelynienaber commented Jun 4, 2021 • edited Loading

lauren-li commented Jun 4, 2021 • edited Loading

lauren-li left a comment

Choose a reason for hiding this comment

lauren-li Jun 4, 2021

Choose a reason for hiding this comment

std4lqi Jun 7, 2021

Choose a reason for hiding this comment

lauren-li Jun 8, 2021

Choose a reason for hiding this comment

tiantn commented Jun 7, 2021

katelynienaber left a comment

Choose a reason for hiding this comment

lauren-li left a comment

Choose a reason for hiding this comment

tiantn commented Jun 8, 2021

katelynienaber left a comment

Choose a reason for hiding this comment

katelynienaber commented Jun 4, 2021 •

edited

Loading

lauren-li commented Jun 4, 2021 •

edited

Loading