Skip to content

fix: Remove redundant directory creation and use HttpClient from DownloadOptions#1669

Merged
thomhurst merged 1 commit into
mainfrom
fix/filesystem-fixes
Dec 30, 2025
Merged

fix: Remove redundant directory creation and use HttpClient from DownloadOptions#1669
thomhurst merged 1 commit into
mainfrom
fix/filesystem-fixes

Conversation

@thomhurst
Copy link
Copy Markdown
Owner

Summary

  • FileSystemModuleEstimatedTimeProvider: Remove duplicate directory creation call
  • Downloader: Pass HttpClient from DownloadOptions to HttpOptions

Changes

  • FileSystemModuleEstimatedTimeProvider.cs: Remove redundant directoryInfo.Create() call that was already handled by conditional check above
  • Downloader.cs: Add HttpClient = options.HttpClient to HttpOptions so custom HttpClient instances are actually used

Test plan

  • Build verification passes
  • Existing tests continue to pass

Fixes #1588, fixes #1589

🤖 Generated with Claude Code

@thomhurst
Copy link
Copy Markdown
Owner Author

Summary

This PR fixes two bugs: removes a redundant directory creation call and ensures custom HttpClient instances from DownloadOptions are properly passed through to HttpOptions.

Critical Issues

None found ✅

Analysis

Change 1: Downloader.cs (src/ModularPipelines/Context/Downloader.cs:62)

Bug Fix: The DownloadOptions.HttpClient property was not being passed to HttpOptions, meaning custom HttpClient instances were being ignored. This change correctly passes it through, enabling users to provide custom HttpClient configurations (timeouts, handlers, certificates, etc.).

Change 2: FileSystemModuleEstimatedTimeProvider.cs (src/ModularPipelines/Engine/FileSystemModuleEstimatedTimeProvider.cs:30-33)

Code Quality Fix: Removes duplicate directoryInfo.Create() call. The conditional check on lines 28-31 already handles directory creation, making the duplicate call on line 33 redundant.

Both changes are straightforward, focused, and correct.

Verdict

APPROVE - No critical issues

…loadOptions

- FileSystemModuleEstimatedTimeProvider: Remove duplicate directoryInfo.Create()
  call that was redundant since directory is already created in the if block above
- Downloader: Pass HttpClient from DownloadOptions to HttpOptions so that
  custom HttpClient instances are actually used during downloads

Fixes #1588, #1589

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@thomhurst
Copy link
Copy Markdown
Owner Author

Summary

This PR fixes a duplicate directory creation call and passes through the HttpClient option in the Downloader class.

Critical Issues

None found ✅

Suggestions

None - both fixes are straightforward improvements:

  1. FileSystemModuleEstimatedTimeProvider.cs:33 - Removes duplicate directoryInfo.Create() call (redundant after line 30)
  2. Downloader.cs:63 - Adds missing HttpClient = options.HttpClient property mapping to respect the HttpClient option from DownloadOptions

Verdict

APPROVE - No critical issues

@thomhurst thomhurst merged commit 5369493 into main Dec 30, 2025
10 of 12 checks passed
@thomhurst thomhurst deleted the fix/filesystem-fixes branch December 30, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant