Skip to content

Conversation

aayushchouhan09
Copy link
Member

@aayushchouhan09 aayushchouhan09 commented Oct 17, 2025

Describe the Problem

Cache bucket uploads were failing with error "Invalid last_modified_time returned from hub_promise" when using S3 namespace as the hub. The cache upload would abort even though the hub upload succeeded, preventing objects from being cached locally.

Explain the Changes

  1. Fixed field name mismatch in namespace_cache.js changed upload_res.last_modified_time to upload_res.date to match the field name returned by namespace_s3.js hub upload response.

Issues: Fixed #xxx / Gap #xxx

  1. Bug fixed: https://issues.redhat.com/browse/DFBUGS-4148
  2. PR (GAP): SDK | Upgrade AWS SDK to v3 - NamespaceS3 #9163

Testing Instructions:

  1. Create aws namespacestore bucket with caching upload some object to the bucket and verify if the objects are available in the cache using list_objects noobaa api.
  • Doc added/updated
  • Tests added

Summary by CodeRabbit

  • Bug Fixes
    • Fixed timestamp retrieval during cache uploads so the stored modification time now comes from the correct response field.
  • Tests
    • Updated unit tests to reflect the revised upload response format and ensure cache timestamps are validated correctly.

Copy link

coderabbitai bot commented Oct 17, 2025

Walkthrough

Changed the cache-upload path to read the object's last-modified time from upload_res.date instead of upload_res.last_modified_time during uploads; NaN validation and all other logic remain unchanged.

Changes

Cohort / File(s) Summary
Cache upload response property
src/sdk/namespace_cache.js, src/test/unit_tests/internal/test_namespace_cache.js
Switched the upload response field used for last-modified time from last_modified_time to date. Updated tests/mocks to return date and assert cache_obj.create_time against ret.date; NaN check and existing branches unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

size/S

Suggested reviewers

  • alphaprinz

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Added a fix for S3 namespace bucket caching while upload" accurately identifies the main area of change in the changeset. The PR does fix a specific issue related to S3 namespace bucket caching during upload operations—specifically, a field name mismatch where the code was reading the wrong response field. While the title could be more specific about the technical details of the fix (changing from last_modified_time to date), it clearly summarizes the primary intent and scope of the changeset without being misleading or overly vague.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b3fe966 and e3a94c8.

📒 Files selected for processing (2)
  • src/sdk/namespace_cache.js (1 hunks)
  • src/test/unit_tests/internal/test_namespace_cache.js (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/test/**/*.*

⚙️ CodeRabbit configuration file

src/test/**/*.*: Ensure that the PR includes tests for the changes.

Files:

  • src/test/unit_tests/internal/test_namespace_cache.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Noobaa Image
  • GitHub Check: run-jest-unit-tests
  • GitHub Check: run-package-lock-validation
🔇 Additional comments (3)
src/test/unit_tests/internal/test_namespace_cache.js (2)

279-279: LGTM! MockNamespace correctly mirrors namespace_s3.js response structure.

The mock now returns date instead of last_modified_time, accurately simulating the actual S3 namespace hub behavior. Both error recovery and success paths are updated consistently.

Also applies to: 299-299


437-437: LGTM! Test assertion correctly validates the field name change.

The assertion now checks against ret.date, properly verifying that the cached object receives the correct timestamp from the upload response.

src/sdk/namespace_cache.js (1)

667-667: Fix verified—namespace_s3.js upload_object correctly returns date field.

The change in namespace_cache.js line 667 is correct. The upload_object method in namespace_s3.js returns an object with date field, confirming the fix properly resolves the field name mismatch. The NaN validation downstream will catch any invalid dates. No further action needed.


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 and usage tips.

@aayushchouhan09 aayushchouhan09 changed the title Added a fix for namespace bucket caching while upload Added a fix for S3 namespace bucket caching while upload Oct 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants