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.

Sign up for GitHub

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

Jump to bottom

feat: add retry for clamav init #8067

Merged
merged 2 commits into from
Jan 22, 2025
Merged

feat: add retry for clamav init #8067

merged 2 commits into from
Jan 22, 2025

Conversation

kevin9foong
Copy link
Contributor

@kevin9foong kevin9foong commented Jan 22, 2025 •
edited by KenLSM
Loading

Problem

Our virus-scanner sometimes fails randomly.

Solution

Adds retries to re-attempt the upload.

Breaking Changes

  • No - this PR is backwards compatible

Tests

Regression

Non-malicious files does not block submission

  • Create a form with attachments
  • Submit the form with attachments
  • Ensure that form is submitted successfully

Malicious files blocks submission

  • Create a form with attachments
  • Submit the form with malicious attachments (eicar)
  • Ensure that form is submitted not successfully

@kevin9foong kevin9foong requested a review from KenLSM January 22, 2025 08:59
@kevin9foong kevin9foong self-assigned this Jan 22, 2025
@datadog-opengovsg
Copy link

datadog-opengovsg bot commented Jan 22, 2025 •
edited
Loading

Datadog Report

Branch report: fix/retry-clamav
Commit report: 0c90c41
Test service: formsg

✅ 0 Failed, 2 Passed, 0 Skipped, 22.13s Total duration (13m 44.05s time saved)

KenLSM
KenLSM approved these changes Jan 22, 2025
View reviewed changes
Copy link
Contributor

@KenLSM KenLSM left a comment

Choose a reason for hiding this comment

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

LGTM!

@KenLSM KenLSM merged commit 88f38d0 into develop Jan 22, 2025
17 checks passed
@KenLSM KenLSM deleted the fix/retry-clamav branch January 22, 2025 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KenLSM KenLSM KenLSM approved these changes

Assignees

@kevin9foong kevin9foong

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kevin9foong @KenLSM