Constrain VirusScanWorker to one concurrent worker per asset
#1263
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will later be used to ensure we cannot enqueue the same asset twice for virus checking.
brucebolt
force-pushed
the
limit-virus-scan-workers
branch
2 times, most recently
from
4763424 to
f4ee455
Compare
Prior to version 7 of `sidekiq-unique-jobs`, the middleware was configured automatically. It now has to be added to an initialiser.
This will allow us to test whether sidekiq jobs have been correctly queued.
This will prevent multiple jobs becoming enqueued for a single asset until the original job has finished executing. We are currently seeing an issue where a user can replace the file after uploading but before the virus scan has completed. This results in a second virus scan being enqueued, which subsequently fails as the asset has been deleted from the filesystem. This will prevent that from happening.
brucebolt
force-pushed
the
limit-virus-scan-workers
branch
from
904d90d to
d0451ad
Compare
…e asset is unscanned" This code won't be needed anymore, since we no longer permit two concurrent workers to be enqueued for the same asset. This reverts commit 4705054.
brucebolt
changed the title
VirusScanWorker to one worker per assetVirusScanWorker to one concurrent worker per asset
aldavidson
approved these changes
Dec 7, 2023
jkempster34
added a commit
that referenced
this pull request
Mar 18, 2024
Currently, when an asset is updated with a file which has the same name as the previous file, the metadata (specifically the MD5 hexdigest) doesn't get updated. There is a guard statement in the S3 uploader that prevents assets with and unchanged MD5 hexdigest) from uploading. This is due to a condition added in #1258 that was intended to quieten some errors we were seeing at the time. This same issue was addressed in #1263.
jkempster34
added a commit
that referenced
this pull request
Mar 18, 2024
Asset metadata is not being updated when an asset is uploaded with a file which has the same name as the previous file. This is because of a condition added in #1258 that was intended to quieten some errors we were seeing at the time. This same issue was addressed in #1263. This is problematic as there is a guard statement in the S3 uploader that prevents assets with an unchanged MD5 hexdigest from uploading.
jkempster34
added a commit
that referenced
this pull request
Mar 18, 2024
Asset metadata is not being updated when an asset is uploaded with a file which has the same name as the previous file. This is because of a condition added in #1258 that was intended to quieten some errors we were seeing at the time. This same issue was addressed in #1263. This is problematic as there is a guard statement in the S3 uploader that prevents assets with an unchanged MD5 hexdigest from uploading.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will prevent multiple jobs becoming enqueued for a single asset until the original job has finished executing.
We are currently seeing an issue where a user can replace the file after uploading but before the virus scan has completed. This results in a second virus scan being enqueued, which subsequently fails as the asset has been deleted from the filesystem. This should prevent that from happening.
Trello card