Skip to content
This repository has been archived by the owner on May 2, 2022. It is now read-only.

SARAALERT-1480: Resolve ActiveJob::DeserializationError Download not found error #1076

Merged
merged 1 commit into from
Sep 20, 2021

Conversation

Bialogs
Copy link
Contributor

@Bialogs Bialogs commented Sep 13, 2021

Description

Jira Ticket: SARAALERT-1480

This error appears when the Export Retry time is set lower than the Download Destroy Wait time. This configuration allows a User to perform another export before the DownloadsDestroyJob is run, causing the DeserializationError. When the User performs another export, the previous exports of the same type are destroyed. This causes the DeserializationError at most 45 minutes later.

As before, even if the Download is not found for a different reason, it will be destroyed by the PurgeJob and not lingering in object storage.

(Bugfix) How to Replicate

  1. Set the admin setting for exports lower than the download destroy wait time
  2. Perform an export and download the results
  3. Perform another export of the same type once the export timer has passed
  4. Wait for the destroy downloads job to be performed (this should error)

(Bugfix) Solution

  • Convert the Worker's perform call to use the Download ID instead of object so that exists can be called before destroy.

Important Changes

Please list important files (meaning substantial or integral to the PR) along with a list of the general changes that should be highlighted for reviewers.

proxy_controller.rb

  • Pass ID instead of Model

destroy_downloads_job.rb

  • Check if Download exists before continuing

Testing Recommendations

Please list any recommendations regarding what reviewers should test and if there is any specific guidance on how to test certain aspects of the PR. Be sure to mention edge cases that should be tried, particularly in the UI.

Checklists

Submitter:

  • This PR describes why these changes were made.
  • This PR is into the correct branch.
  • This PR includes the correct JIRA ticket reference.
  • Comment added to the relevant JIRA ticket(s) with a link to this PR
  • Code diff has been reviewed (it does not contain: additional white space, not applicable code changes, debug statements, etc.)
  • If UI changes have been made, Chrome Dev Tools Lighthouse accessibility test has been executed to ensure no 508 issues were introduced.
  • Tests are included and test edge cases
  • Tests have been run locally and pass (remember to update Gemfile when applicable)
  • Test fixtures updated and documented as necessary

@timwongj :

  • Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
  • The tests appropriately test the new code, including edge cases
  • You have tried to break the code
  • If applicable, you have tested changes against a large database, and considered possible performance regressions
  • Tested all recommendations listed in the "Testing Recommendations" section. The application behaves as expected with this PR.

@tstrass :

  • Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
  • The tests appropriately test the new code, including edge cases
  • You have tried to break the code
  • If applicable, you have tested changes against a large database, and considered possible performance regressions
  • Tested all recommendations listed in the "Testing Recommendations" section. The application behaves as expected with this PR.

@ngfreiter :

  • Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
  • The tests appropriately test the new code, including edge cases
  • You have tried to break the code
  • If applicable, you have tested changes against a large database, and considered possible performance regressions
  • Tested all recommendations listed in the "Testing Recommendations" section. The application behaves as expected with this PR.

@Bialogs Bialogs changed the base branch from master to 1.37.0 September 13, 2021 18:28
@Bialogs Bialogs force-pushed the 1480-destroy_downloads_job_error branch 2 times, most recently from 4d1ac07 to 35c29a7 Compare September 13, 2021 19:43
This error appears when the Export Retry time is set lower than the Download Destroy Wait time. This configuration allows a User to perform another export before the DownloadsDestroyJob is run, causing the DeserializationError. When the User performs another export, the previous exports of the same type are destroyed. This causes the DeserializationError at most 45 minutes later.

As before, even if the Download is not found for a different reason, it will be destroyed by the PurgeJob and not lingering in object storage.

* Convert the Worker's perform call to use the Download ID instead of object so that a Download#exists? check can be performed before attempting to destroy.
Copy link
Contributor

@tstrass tstrass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@tstrass tstrass merged commit cf0f2d3 into 1.37.0 Sep 20, 2021
@tstrass tstrass deleted the 1480-destroy_downloads_job_error branch September 20, 2021 20:32
tstrass pushed a commit that referenced this pull request Sep 27, 2021
…found error (#1076)

This error appears when the Export Retry time is set lower than the Download Destroy Wait time. This configuration allows a User to perform another export before the DownloadsDestroyJob is run, causing the DeserializationError. When the User performs another export, the previous exports of the same type are destroyed. This causes the DeserializationError at most 45 minutes later.

As before, even if the Download is not found for a different reason, it will be destroyed by the PurgeJob and not lingering in object storage.

* Convert the Worker's perform call to use the Download ID instead of object so that a Download#exists? check can be performed before attempting to destroy.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants