Skip to content

#OI-I3: Fix: Mandatory header required during azure blob upload#367

Merged
manjudr merged 1 commit intomainfrom
api-svc-fixes
Dec 8, 2025
Merged

#OI-I3: Fix: Mandatory header required during azure blob upload#367
manjudr merged 1 commit intomainfrom
api-svc-fixes

Conversation

@JeraldJF
Copy link
Collaborator

@JeraldJF JeraldJF commented Dec 8, 2025

Summary by CodeRabbit

  • New Features
    • Enhanced cloud storage upload functionality with comprehensive Azure blob storage support.
    • Optimized header configuration and management for improved file transfer reliability.
    • Added new Azure-specific configuration settings for better cloud storage compatibility.
    • Improved upload flow to ensure enhanced support across different cloud storage providers.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 8, 2025

Walkthrough

The changes add Azure Blob Storage type configuration and enhance the upload flow to construct and pass custom headers during file uploads, with conditional support for Azure-specific blob type headers based on the cloud storage provider.

Changes

Cohort / File(s) Summary
Configuration
api-service/src/configs/Config.ts
Added azure_blob_type field under cloud_config with default value "BlockBlob"; minor whitespace adjustments and formatting tweaks.
Upload Controller
api-service/src/controllers/ConnectorRegister/ConnectorRegisterController.ts
Imported config module; enhanced upload flow to construct uploadHeaders with Content-Type and Content-Length; conditionally add x-ms-blob-type header when cloud storage provider is Azure; refactored axios.put call to use constructed headers.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Controller as ConnectorRegister<br/>Controller
    participant Config
    participant Azure as Azure Blob<br/>Storage

    Client->>Controller: Upload request
    Controller->>Config: Read cloud config<br/>(provider, azure_blob_type)
    Config-->>Controller: Config data

    Controller->>Controller: Construct uploadHeaders<br/>(Content-Type, Content-Length)
    
    alt Azure Provider
        Controller->>Controller: Add x-ms-blob-type header<br/>from config
    end
    
    Controller->>Azure: axios.put(url, data,<br/>uploadHeaders)
    Azure-->>Controller: Upload response
    Controller-->>Client: Return result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas requiring attention:
    • Verify that azure_blob_type configuration correctly maps to Azure SDK expectations
    • Ensure conditional header injection for Azure doesn't affect other cloud providers
    • Confirm upload header construction maintains backward compatibility with existing upload paths

Poem

🐰 A config hops in, azure blooms so true,
Headers dress up files for Azure's view,
Conditional wisdom guides the way,
Upload flows dance in a brand new way! 🚀

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 clearly and specifically describes the main functional change: adding a mandatory Azure blob header to the upload flow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch api-svc-fixes

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
api-service/src/configs/Config.ts (1)

95-106: Azure blob type config is wired correctly; consider clarifying/validating expected values.

The new cloud_config.azure_blob_type defaulting to "BlockBlob" is a good fit for Azure uploads and aligns with how it’s used in ConnectorRegisterController.ts. To avoid subtle misconfig issues later, you may want to:

  • Document the allowed values (BlockBlob, AppendBlob, PageBlob, etc.) alongside this config, and/or
  • Add lightweight validation where this is consumed so obviously invalid values fail fast.

Functionally this looks good as-is.

api-service/src/controllers/ConnectorRegister/ConnectorRegisterController.ts (1)

12-13: Azure upload headers look correct; consider normalizing the provider name.

The new uploadHeaders (Content-Type + Content-Length) and conditional addition of x-ms-blob-type from config.cloud_config.azure_blob_type when cloud_storage_provider === "azure" align well with Azure’s requirement for this header and keep other providers unaffected.

One minor robustness tweak you might consider:

-        if (config.cloud_config.cloud_storage_provider === "azure") {
+        if (config.cloud_config.cloud_storage_provider?.toLowerCase() === "azure") {
             uploadHeaders["x-ms-blob-type"] = config.cloud_config.azure_blob_type;
         }

This avoids issues if cloud_storage_provider is configured as "Azure" / "AZURE" while still keeping behavior unchanged for other values.

Overall, the header construction and usage in axios.put look good.

Also applies to: 81-92

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d5a757 and 0f6f9a4.

📒 Files selected for processing (2)
  • api-service/src/configs/Config.ts (4 hunks)
  • api-service/src/controllers/ConnectorRegister/ConnectorRegisterController.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
api-service/src/controllers/ConnectorRegister/ConnectorRegisterController.ts (1)
api-service/src/configs/Config.ts (1)
  • config (20-154)
🔇 Additional comments (1)
api-service/src/controllers/ConnectorRegister/ConnectorRegisterController.ts (1)

27-34: Early failure when signed URL is missing is a solid improvement.

The explicit check on urlPayload.download_url and throwing SIGNED_URL_NOT_FOUND with a 400 status avoids proceeding with an invalid URL and makes the failure mode clearer to callers. No issues from my side here.

@manjudr
Copy link
Member

manjudr commented Dec 8, 2025

The changes looks fine

@manjudr manjudr merged commit d14a20b into main Dec 8, 2025
7 of 8 checks passed
@manjudr manjudr deleted the api-svc-fixes branch December 8, 2025 10:29
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