-
Notifications
You must be signed in to change notification settings - Fork 92
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
Fix Etag Mismatch auto saving #2316
Conversation
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
Fix zowe.settings.version being added to workspace settings
Signed-off-by: zowe-robot <zowe.robot@gmail.com>
Signed-off-by: zowe-robot <zowe.robot@gmail.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2316 +/- ##
==========================================
+ Coverage 91.90% 92.05% +0.14%
==========================================
Files 90 90
Lines 9242 9222 -20
Branches 1908 1906 -2
==========================================
- Hits 8494 8489 -5
+ Misses 747 732 -15
Partials 1 1
☔ View full report in Codecov by Sentry. |
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @JillieBeanSim 🙂 just had a question regarding implementation for putContents
result.success = false; | ||
result.commandResponse = "Rest API failure with HTTP(S) status 412 Save conflict."; | ||
return result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that the MVS API returns an unsuccessful result without throwing an error, but the USS API throws an error. I see that we are catching errors when calling this function, but should we maintain the same behavior/return value for when save conflicts appear in putContents
across all FTP APIs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @traeok the FTP extension is just following the same protocol used with zosmf, I did notice USS and MVS are handled differently but that is how zowe cli was returning these and Zowe Explorer was built around that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this and it seems to be handling the conflicts and displaying the diff view on VS Code to handle saving mismatches, thanks @JillieBeanSim !
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm consistently getting the error that this PR fixes:
will re-review after that one
Changes LGTM though.
there will be a very small conflict with this other PR:
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 😋
now we get the compare button for both FTP and ZE 🥳
Proposed changes
fix for #2277
Remove duplicate code and updated the handling of dirty file when etag mismatch error for MVS and z/OS UNIX files by marking file unsaved before pulling the latest changes to the local file then use the vscode built in command
workspace.files.action.compareWithSaved
.This PR was originally based against maintenance so will also bring in the commits for 2.8.2 fix & release
Fixed the error returned by FTP ext to trigger the diff editor with mismatch etag.
Release Notes
Milestone: 2.9.0
Changelog:
ZE: Fixed an issue with mismatch etag error returned not triggering the diff editor and possible loss of data due to the issue. #2277
zFTP: - Fixed an issue with mismatch etag, correcting error message sent to Zowe Explorer to trigger diff editor. #2277
Types of changes
What types of changes does your code introduce to Zowe Explorer?
Put an
x
in the boxes that applyChecklist
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 revieweryarn workspace vscode-extension-for-zowe vscode:prepublish
has been executedFurther comments
Theia:
zFTP: