Skip to content

Fix checksum from file content#290

Merged
soimkim merged 2 commits into
mainfrom
cheksum
Jul 3, 2026
Merged

Fix checksum from file content#290
soimkim merged 2 commits into
mainfrom
cheksum

Conversation

@dd-jy

@dd-jy dd-jy commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Improved checksum generation: SHA-1 is now computed only when the provided source points to an existing file.
    • When the file can’t be opened/read or an error occurs, the app logs the issue and safely falls back to a null checksum value instead of interrupting normal operation.

@dd-jy dd-jy self-assigned this Jul 3, 2026
@dd-jy dd-jy added the bug fix [PR] Fix the bug label Jul 3, 2026
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@dd-jy, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fc7b6ce5-2930-44a7-9831-5e2ab6564cf8

📥 Commits

Reviewing files that changed from the base of the PR and between 0b07abb and e6895c9.

📒 Files selected for processing (1)
  • src/fosslight_util/oss_item.py
📝 Walkthrough

Walkthrough

get_checksum_sha1 in src/fosslight_util/oss_item.py now hashes only existing files by reading them in binary mode, and on error logs at debug level while leaving the checksum at CHECKSUM_NULL.

Changes

Checksum Computation Update

Layer / File(s) Summary
Update checksum computation logic
src/fosslight_util/oss_item.py
get_checksum_sha1 now hashes binary file contents only when source_name_or_path is an existing file, and exceptions leave the default checksum value in place.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: fixing checksum computation to use file content in oss_item.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cheksum

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/fosslight_util/oss_item.py (1)

180-182: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid loading entire file into memory before hashing.

f.read() buffers the whole file before hashing. This function is invoked for FOSSLIGHT_BINARY/FOSSLIGHT_SOURCE scan items, which can include large binaries — reading the entire content into memory is wasteful and risks high peak memory usage for large files. Consider streaming in chunks.

♻️ Proposed chunked read
     try:
         with open(source_name_or_path, "rb") as f:
-            byte = f.read()
-            checksum = str(hashlib.sha1(byte).hexdigest())
+            sha1 = hashlib.sha1()
+            for chunk in iter(lambda: f.read(65536), b""):
+                sha1.update(chunk)
+            checksum = sha1.hexdigest()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/fosslight_util/oss_item.py` around lines 180 - 182, The checksum logic in
the file-hash helper currently reads the entire source_name_or_path into memory
before computing the SHA1, which is risky for large FOSSLIGHT_BINARY and
FOSSLIGHT_SOURCE items. Update the hashing flow in the relevant method in
oss_item.py to stream the file in chunks instead of using a single full read,
while preserving the existing SHA1 result and call sites that rely on it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/fosslight_util/oss_item.py`:
- Around line 180-182: The checksum logic in the file-hash helper currently
reads the entire source_name_or_path into memory before computing the SHA1,
which is risky for large FOSSLIGHT_BINARY and FOSSLIGHT_SOURCE items. Update the
hashing flow in the relevant method in oss_item.py to stream the file in chunks
instead of using a single full read, while preserving the existing SHA1 result
and call sites that rely on it.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8bd56354-5e0c-4bfb-835c-231ba6110764

📥 Commits

Reviewing files that changed from the base of the PR and between 5935171 and 52482bd.

📒 Files selected for processing (1)
  • src/fosslight_util/oss_item.py

@soimkim

soimkim commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@dd-jy , 파일 존재 여부 체크 후 checksum 시도하게 해주십시오.
_logger.info 보다 _logger_debug 제안드립니다. 출력 안 되어도 risk가 크지 않고 사용자가 info 메세지를 확인하고 뭔가를 할 필요는 없어 보여서요.

@soimkim

soimkim commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@dd-jy , 대용량 파일에 대한 메모리 사용량을 줄이기 위해 전체 read 대신 chunk 단위(1MB)로 SHA1 업데이트하도록 변경 제안드립니다.

+_CHECKSUM_CHUNK_SIZE = 1024 * 1024  # 1 MiB
-        with open(file_path, 'rb') as f:
-            checksum = hashlib.sha1(f.read()).hexdigest()
+        sha1 = hashlib.sha1()
+        with open(file_path, 'rb') as f:
+            for chunk in iter(lambda: f.read(_CHECKSUM_CHUNK_SIZE), b''):
+                sha1.update(chunk)
+        checksum = sha1.hexdigest()

Signed-off-by: 석지영/책임연구원/SW공학(연)Open Source TP <jiyeong.seok@lge.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/fosslight_util/oss_item.py`:
- Around line 180-187: The checksum logic in get_checksum still reads the entire
file into memory via f.read(), which needs to be replaced with incremental SHA1
hashing for large files. Update the file-handling branch in get_checksum to read
source_name_or_path in fixed-size chunks (use the requested 1 MiB chunk size)
and feed each chunk into hashlib.sha1.update(), while keeping the existing
checksum return and exception handling behavior intact.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 590ed2f9-b5f5-480d-bfc6-864b64e10286

📥 Commits

Reviewing files that changed from the base of the PR and between 52482bd and 0b07abb.

📒 Files selected for processing (1)
  • src/fosslight_util/oss_item.py

Comment thread src/fosslight_util/oss_item.py
Signed-off-by: 석지영/책임연구원/SW공학(연)Open Source TP <jiyeong.seok@lge.com>
@soimkim
soimkim merged commit 4728df4 into main Jul 3, 2026
8 checks passed
@soimkim
soimkim deleted the cheksum branch July 3, 2026 05:45
@soimkim soimkim changed the title fix(oss_item): fix checksum from file content Fix checksum from file content Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug fix [PR] Fix the bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants