Skip to content

Conversation

@honeynil
Copy link
Contributor

Fix HTTP client parameter bug and improve disk storage consistency

Description

This PR fixes two bugs found in the codebase:

  1. HTTP Storage Client Bug: The NewHTTP function in internal/infra/storage/http.go was ignoring the client parameter and always using http.DefaultClient. This prevented users from configuring custom HTTP clients with specific timeouts or transport settings.

  2. Disk Storage Consistency Issue: The writeMeta function in internal/infra/storage/disk.go was using O_TRUNC flag instead of O_EXCL, which made it inconsistent with writeFile and prevented the retryOnExist helper from working correctly for metadata files.

Changes

  • internal/infra/storage/http.go: Changed line 21 to use the provided client parameter instead of http.DefaultClient
  • internal/infra/storage/disk.go: Changed line 338 to use O_EXCL flag instead of O_TRUNC for consistency

Impact

  • HTTP storage can now properly use custom HTTP clients with configured timeouts and transports
  • Disk storage retry logic now works correctly for metadata files
  • Code is more consistent and maintainable
  • No breaking changes - all existing tests pass

TODOs

Read the Contribution guidelines.

  • Generate the docs. (No doc changes needed - internal implementation detail)
  • Run the relevant tests successfully. (All tests pass)
  • Include release notes. (See below)

Release Notes (draft)

Fixed HTTP client parameter being ignored in NewHTTP function and improved disk storage consistency by using O_EXCL flag in writeMeta.

Migration Guide

No migration needed - these are bug fixes with no API changes.

…ing http.DefaultClient. Change writeMeta to use O_EXCL flag instead of O_TRUNC for consistency with writeFile
@xakep666
Copy link
Collaborator

@honeynil thank you for fix!

@xakep666 xakep666 merged commit c8c85a3 into platacard:main Nov 28, 2025
4 checks passed
@honeynil honeynil deleted the fix/http-client-and-disk-storage-consistency branch November 28, 2025 12:30
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