Skip to content

Conversation

jstucke
Copy link
Collaborator

@jstucke jstucke commented Apr 11, 2025

The BinaryService needs a DB interface to retrieve the file name. This...

  • leads to unnecessary DB accesses, because most of the time the file name is actually not used
  • this can lead to errors if there are multiple requests at once (because it is accessed from a threaded "intercom backend responder" and the DB session there is not thread safe)
  • is kinda unnecessary anyway because in all cases where the file name is actually needed, we have access to a DB interface anyway and can simple use that to look it up

This PR removes DB access from BinaryService. There is now no real reason to have a separate "BinaryService" and "FSOrganizer" because they are closely related (the FSO stores files and the BS reads them), so we merge both and called the result "FileService".

@jstucke jstucke self-assigned this Apr 11, 2025
@jstucke jstucke force-pushed the remove-binary-service branch from 19f9288 to aeacd77 Compare April 29, 2025 11:00
@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2025

Codecov Report

❌ Patch coverage is 96.38554% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.89%. Comparing base (a392b42) to head (0e4d608).

Files with missing lines Patch % Lines
...hitecture_detection/code/architecture_detection.py 60.00% 2 Missing ⚠️
src/test/unit/conftest.py 71.42% 2 Missing ⚠️
src/scheduler/analysis/scheduler.py 66.66% 1 Missing ⚠️
src/storage/file_service.py 97.82% 1 Missing ⚠️
...st/integration/intercom/test_task_communication.py 95.65% 1 Missing ⚠️
src/test/unit/web_interface/test_app_download.py 83.33% 1 Missing ⚠️
src/web_interface/components/analysis_routes.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1363      +/-   ##
==========================================
- Coverage   91.90%   91.89%   -0.02%     
==========================================
  Files         372      370       -2     
  Lines       20955    20948       -7     
==========================================
- Hits        19259    19250       -9     
- Misses       1696     1698       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jstucke jstucke force-pushed the remove-binary-service branch from 3acf8eb to 3ccb40f Compare May 16, 2025 16:11
@jstucke jstucke force-pushed the remove-binary-service branch from 3ccb40f to 0e4d608 Compare August 20, 2025 08:51
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