Vscode-extension-for-zowe: Opening files. Download new copy or reuse local?

[Colin-Stone]

One concern that has been raised to me regarding the editor is that when opening a file, Zowe Explorer always tries to use an existing local version of a dataset or USS file rather than download a fresh version of the file. Although this is fine for files that don't change, if you are looking at a file that frequently changes it may be preferable to download every time.

We do provide the download icon to override the "already exists check" but apart from very big files is it a massive overhead to download each and everytime the user clicks on the file name in the explorer?.

Options are

1. Add a setting with the default being download always. (or not)
2. Base the decision on file size. If relatively small always download.

[MikeBauerCA]

My opinion is that clicking the name of the data-set or USS file in the tree would download a fresh version of the file given that it isn't currently being edited in a VSCode frame. When I start editing a file, I would like to start with the latest version to reduce possible merge conflicts.

However, we need to keep in mind the use case where someone is using the tree to navigate between files while editing. I would not want my local changes to be overridden in that case. Personally though, I do find it odd that the Explorer tries to use a local version even after the local version has been closed in the editor.

[jelaplan]

Could you open the local file and concurrently do a diff check on the remote file and if a diff is found, notify the user?

[stepanzharychevbroadcom]

@Colin-Stone @jelaplan I analysed what we currently have for this problem, the situation is we already have implemented conflict solver which analyses ETags. The only case which should be covered looks like this:

1. User has local copy of file downloaded and closed in explorer;
2. User opens the file; <=== That's a place where potential auto-download should happen
3. User edits the file;
4. User tries to save and gets unnecessary merge conflict, because file was modified before he actually opened it;

So for (2) I think we need to pre-download the file before user actually sees it to override the saved content. But it should be done wisely taking into account the following rules:
1) We should check that file actually was changed on mainframe side, it can be done via this method using If-None-Match header, so we won't download anything in the case if local ETag matches remote one;
2) We should limit size of file which is suitable for such auto-update (maybe 10 Mb for start);
3) If file wasn't updated for certain amount of time and there's a local copy, we should probably show special sign for user, so he knows that may be it's a good time to update (I will detail this suggestion in separate issue about explorer for USS);

Please, let me know about your opinion.

[Colin-Stone]

Hi Stepan. I had a suggestion from a user today that if a they have closed a file that it should always warrant reloading it if opened again. The rationale being that if he wanted to look at it again in it's current state he would have left it open. It's sort of logical and creates less surprise I think. I would also apply that rule to +10MB rather than add too many caveats

[stepanzharychevbroadcom]

Hey @Colin-Stone, that's exactly what I tried to describe. Point (1) is just for warranting us that download will be done only when file was really changed on the mainframe side.
Can you, please, clarify what's exactly the problem with 10Mb rule to your mind?

[Colin-Stone]

If you have downloaded a big +10MB file and subsequently closed it. I would expect that by reopening it to get a new copy downloaded rather than the one I closed earlier. I think we may be lucky with machines that are pretty quick so I appeciate this may not be the case for everyone. Saving a local copy was done for performance reasons but I have aways felt it confuses more users than the extra time downloading annoys users. What does everyone else reckon?.

[stepanzharychevbroadcom]

@Colin-Stone I think we still can compare ETag, but we should definitely ask user if he wants to download such files. Not everyone has great internet connection and traffic might matter here.

[stepanzharychevbroadcom]

Also, as I promised I tried to formulate other connected enhancements which can be done before this one: https://github.com/zowe/vscode-extension-for-zowe/issues/461

[Colin-Stone]

Considering again and unless we get further ideas I am happy if you go ahead with your original suggestions.

[stepanzharychevbroadcom]

@Colin-Stone I will assign it to myself then.

[zFernand0]

I think we still can compare ETag, but we should definitely ask user if he wants to download such files. Not everyone has great internet connection and traffic might matter here.

Checking to see if the ETag changed sounds like a good idea.
It seems that when retrieving the contents, we can provide the If-None-Match header and handle the HTTP 304 Not Modified response to use the local file?
@Alexandru-Dumitru, any thoughts?

[venkatzhub]

Was the classic approach of comparing timestamps to decide whether to download or not considered?

[Alexandru-Dumitru]

I think we still can compare ETag, but we should definitely ask user if he wants to download such files. Not everyone has great internet connection and traffic might matter here.

Checking to see if the ETag changed sounds like a good idea.
It seems that when retrieving the contents, we can provide the If-None-Match header and handle the HTTP 304 Not Modified response to use the local file?
@Alexandru-Dumitru, any thoughts?

Yes, we can use it. As I discussed with @stepanzharychevbroadcom, it would save us some unnecessary downloads.

Was the classic approach of comparing timestamps to decide whether to download or not considered?

I would see how we can benefit from this approach as well, as we would only use a list call requesting the necessary attributes. We would need to make a PoC to use mtime instead of etag to make sure it covers our use cases. Also list would be less resource hungry when we discuss larger files.

For example, currently the merge conflict is generated as follows:

| PC | REST Call | MF |
| ------------- | ------------- | --------- |
| file1 | <---etag(1)----(download) | file1 (etag1) |
| file1(etag1) | | file1 (etag2) get's modified |
| file1(etag1) | (upload) ------etag(1)----> | file1 (etag2) Req. denied ! eta1 != eta2 |
| file1(etag1) | <----etag(2)---(download) | file1 (etag2) |
| file1(etag1) < merge conflict> file1(etag2)* | | |
| file1(etag2) | (upload) ----(etag2) ---> | file1(etag2) Req. accepted |

*this is generated by rewriting the content of the file on disk, without modifying the content of the file displayed by VS Code, then calling Save() function of VS Code. At this point, VS Code detects the content on disk is different, and generates the merge conflict panels. Upon resolving, we try to upload again with the new etag, thus going through the same process all over.

[zFernand0]

Was the classic approach of comparing timestamps to decide whether to download or not considered?

I believe that ETag will serve the same purpose as a timestamp comparison : )

[venkatzhub]

@Alexandru-Dumitru - this problem has been solved numerous times using the timestamps (translates to - approach is proved in the field and a variety of real customers shops), and to your point that approach has not been resource intensive, however I don't know how it compares to ETag. So a quick POC would be worthwhile.

[dkelosky]

@Alexandru-Dumitru is your PoC proposal only for the purpose of getting a more up-to-date local copy?

It seems the Save() scenario would still require the use of ETag. Do you agree?

[Alexandru-Dumitru]

@Alexandru-Dumitru is your PoC proposal only for the purpose of getting a more up-to-date local copy?

It seems the Save() scenario would still require the use of ETag. Do you agree?

@dkelosky I was thinking to look into the possibility to replace etag with mtime, to see if it's worth while, or some combination of the two.

[stepanzharychevbroadcom]

@Alexandru-Dumitru I don't think that mtime is reliable here, I still can think about the scenario where do unnecessary download. The case is:

1) User has file (let's call it version#1) which is similar by mtime with mainframe;
2) Other user modifies the same file on the mainframe side once (version#2), but after he changes it back to version#1;
3) User opens this same file which is identical to his local copy and does unnecessary download, because mtime is different;

So I guess the best one would be still to extend Zowe CLI with ETag check and download only if local and remote don't match.

[venkatzhub]

@Stepan Zharychev - while that is a legit scenario - how often does that happen in the real world?

[dkelosky]

@venkatzhub - Agreed it may not happen often and worst case there is an unnecessary download.

For upload (Save()), it seems there could be a window for an unintentional overlay if ETag is not used.

In the client code with an mtime only implementation and during a Save() operation, we could compare times and determine to upload (which is a z/OSMF REST to write).

Behind the scenes this starts a TSO address space. Depending on workload and policy, this could take some time, during which, the file could be altered via ISPF or some other means. A completed upload after this could overwrite content.

Perhaps this would not occur often, but the consequences might be a bit more severe. What do you think?

[Alexandru-Dumitru]

Going through https://tools.ietf.org/html/rfc7232 (HTTP conditional requests) and z/OSMF documentation and it seems that z/OSMF implements only strong validation with entity tags. There is no header to pass _Last-Modified Dates_ to handle , meaning that all the validation logic should be taken care by us (the client). I am not objecting this, just wanted to highlight it.

[venkatzhub]

@dkelosky - the consequence of overwriting are indeed a bit more severe, however the chances are still low. But using ETag on Save is worthwhile to prevent any possible loss of data cases.

[dkelosky]

Hi @Alexandru-Dumitru - sorry but I don't fully understand the link. z/OSMF implements strong validation but validation should be taken care of by the client?

[Alexandru-Dumitru]

Hi @Alexandru-Dumitru - sorry but I don't fully understand the link. z/OSMF implements strong validation but validation should be taken care of by the client?

Sorry for the confusion created. I meant that z/OSMF only implements Etags to address conditional processing of HTTP requests. They have the option to supply and accept mtime as well (mentioned in the RFC) by returning it for the resources in case and also accept it back within some headers, but it is not there. As a result, we would have to implement the request, validation and conditional processing of mtime in the client (zowe/cli) and not to count on z/OSMF for this. With this being said, I would like to side with etag going forward.

[jellypuno]

@stepanzharychevbroadcom is this fixed? if yes, can you please close the issue?

[stepanzharychevbroadcom]

@jellypuno It's still pending. So let's keep it for now.

[travatine]

Hi,

+1 for adding a preference " always download" with default "on".

[zFernand0]

Hi,

+1 for adding a preference " always download" with default "on".

Hey @travatine, (and everyone on the thread)
Feel free to vote here: https://github.com/zowe/vscode-extension-for-zowe/issues/663

[Colin-Stone]

Adding a note that with Jobs. If it is Active, it should always have everything downloaded again

[zdmullen]

We want to implement the following approach, following the poll:

Add a Setting. Default: Always Download