-
-
Notifications
You must be signed in to change notification settings - Fork 9
Finish HT-side implementation of sync-interrupted scenario (HT-508) #338
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
base: master
Are you sure you want to change the base?
Finish HT-side implementation of sync-interrupted scenario (HT-508) #338
Conversation
…eting The substantive changes will only be a few lines. This is a safety commit after the first time both normal and cancel scenarios work. The extra debug output code is still in place and will be removed next, prior to pull_request.
For clarity in assessing the imminent pull request, put back as much as possible of the original code. This will help the actual changes stand out better.
Formerly HT sent HTA "sync_success" regardless of whether 'mergeCompleted' was True. Now the False case leads to "sync_interrupted" being sent to HTA so the user can know that sync did not complete successfully.
|
This is the idea, but I think that in order for it to be useful, we need to update HTA also. |
|
Yes, I just submitted a PR for HTA as well. |
|
Yeah, I saw that right after making the comment. I do think we need to consider (and test) the interaction during the transition when people may have the new HT or the new HTA but not the new version of the other. For example, we may need to send the new message followed by the old one in order to keep the old version of HTA from getting into a stuck state. And, if so, then the new HTA might need to know to ignore the |
|
Great point, I agree. Will address. |
|
I have tried to address, but do not see an HT-only solution. HTA currently ignores the "sync_xxxx" from HT in the HttpRequest and always puts up "Sync completed successfully". Attached is a brief document that considers the four combinations of new/old code. |
|
Here's a Google doc version of the analysis. |
tombogle
left a comment
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.
@tombogle reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @wmergenthal)
|
The changes (at least so far) look good, but see my comments in the doc. I want to keep this as a draft and not merge it until we're ready to release the new HTA app (which might want to include some other compliance-related updates), so we can try to release the two more-or-less imultaneously. |
First cut with addition of minimum HTA version needed for a successful sync. Not yet fully tested.
No substantive changes. Just captures debug stmts producing the output captured in 20251001_02_HT_sync_watchdog_all_ok.log, where watchdog is seen to work correctly for both success and fail scenarios.
Formerly HT sent HTA "sync_success" regardless of whether 'mergeCompleted' was True. Now for the False case HT sends "sync_interrupted" instead to HTA, so the user can know that sync did not complete successfully.
This change is