Skip to content
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

feat: bulk command resiliency improvements #220

Merged
merged 2 commits into from
Oct 3, 2023
Merged

Conversation

danielgtaylor
Copy link
Owner

This change makes some relatively minor changes to help with bulk command resiliency, particularly around the management of the bulk checkout state when dealing with partial failures and killed commands. Combined with #219 retries & timeouts this should now be resilient against HTTP 502 Bad Gateway and HTTP 429 Too Many Requests from rate limiting, as well as HTTP 422 Unprocessable Content on pushes which fail validation, leaving the metadata state in a reasonable place for subsequent commands.

Note that it's still possible to kill the commands mid-run, but rather than having many files not match their metadata you should now e.g. be able to safely kill pulls and pushes with no ill effects or with just one file showing as modified because a write was in progress while it was killed. This is much easier to fix up with a reset than previously.

Specifically, this PR does the following:

  • Bulk pull improvements
    • Save metadata state after each file is fetched (slower but safer)
    • Log errors and continue rather than exiting
    • Log if file deleting fails for removals
  • Bulk push improvements
    • After each file is pushed, mark it as unmodified locally and save the metadata state. If the subsequent fetch is successful it will have its version updated to match the latest on the server. If not, then the next pull should update it automatically.
    • Log errors and continue rather than exiting
    • When finished pushing, only files which were successful are updated to the latest version on the server in the metadata state.
  • Bulk status improvements
    • Order remote changes alphanumerically
  • Bulk diff improvements
    • Log a warning when JSON parsing fails along with which file needs to be fixed

@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Merging #220 (1c467ca) into main (9c15903) will increase coverage by 0.11%.
The diff coverage is 53.19%.

❗ Current head 1c467ca differs from pull request most recent head aeaab4a. Consider uploading reports for the commit aeaab4a to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #220      +/-   ##
==========================================
+ Coverage   76.66%   76.78%   +0.11%     
==========================================
  Files          26       26              
  Lines        3583     3609      +26     
==========================================
+ Hits         2747     2771      +24     
- Misses        636      637       +1     
- Partials      200      201       +1     
Files Coverage Δ
bulk/file.go 70.37% <66.66%> (+5.66%) ⬆️
bulk/commands.go 92.01% <14.28%> (-2.20%) ⬇️
bulk/metadata.go 68.61% <58.82%> (+3.61%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c15903...aeaab4a. Read the comment docs.

@danielgtaylor danielgtaylor merged commit 2c85909 into main Oct 3, 2023
4 checks passed
@danielgtaylor danielgtaylor deleted the bulk-improvements branch October 3, 2023 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants