Skip to content

[API Proposal]: Add FromFile method to BinaryData #106203

Closed

Description

@JoshLove-msft

Background and motivation

Oftentimes, people want to upload local files using BinaryData. In order to do this with the current API, they must write the following code:

var bd = BinaryData.FromStream(File.OpenRead("foo"));

which is not obvious to many users.
It would be better if they could simply do:

var bd = BinaryData.FromFile("foo");

API Proposal

namespace System.Memory.Data;

public partial class BinaryData
{
    public static BinaryData FromFile(string path);
    public static Task<BinaryData> FromFileAsync(string path, CancellationToken cancellationToken = default);
    public static Task<BinaryData> FromFileAsync(string path, string? mediaType, CancellationToken cancellationToken = default);
}

API Usage

var bd = BinaryData.FromFile("foo");

Alternative Designs

No response

Risks

No response

Activity

  1. added
    api-suggestionEarly API idea and discussion, it is NOT ready for implementation
    on Aug 9, 2024
  2. dotnet-policy-service commented on Aug 9, 2024

    @dotnet-policy-service
    Contributor

    Tagging subscribers to this area: @dotnet/area-system-memory
    See info in area-owners.md if you want to be subscribed.

  3. terrajobst commented on Aug 9, 2024

    @terrajobst
    Contributor

    Makes sense.

    Given that FromStream also has FromStreamAsync it seems we'll want a FromFileAsync version as well:

    namespace System.Memory.Data;
    
    public partial class BinaryData
    {
        public static BinaryData FromFile(string path);
        public static Task<BinaryData> FromFileAsync(string path, CancellationToken cancellationToken = default);
        public static Task<BinaryData> FromFileAsync(string path, string? mediaType, CancellationToken cancellationToken = default);
    }
  4. removed
    api-suggestionEarly API idea and discussion, it is NOT ready for implementation
    untriagedNew issue has not been triaged by the area owner
    on Aug 10, 2024
  5. added this to the Future milestone on Aug 10, 2024
  6. skyoxZ commented on Aug 11, 2024

    @skyoxZ
    Contributor
    namespace System.Memory.Data;
    
    public partial class BinaryData
    {
        public static BinaryData FromFile(string path);
        public static Task<BinaryData> FromFileAsync(string path);
    }

    Also more overloads? For mediaType and cancellationToken.

  7. terrajobst commented on Aug 12, 2024

    @terrajobst
    Contributor

    Good call. Fixed.

  8. dersia commented on Aug 13, 2024

    @dersia

    FromFile() should probably also have an overload for mediaType. and is there a need for FromFileAsJson<T>()? this would be very handy, I can also see a FromFileAsGzip() version.

  9. terrajobst commented on Aug 13, 2024

    @terrajobst
    Contributor

    FromFile() should probably also have an overload for mediaType.

    I fixed that, see original comment.

    And is there a need for FromFileAsJson<T>()?

    Maybe? I don't recall ever having that need but that might just be me. What's the scenario for that?

    I can also see a FromFileAsGzip() version

    Today there is no compression handling at all in BinaryData. I think this should be a separate feature request as it seems unrelated to support files.

  10. terrajobst commented on Aug 13, 2024

    @terrajobst
    Contributor

    Video

    • We should also add mediaType to the sync overload
    namespace System.Memory.Data;
    
    public partial class BinaryData
    {
        public static BinaryData FromFile(string path);
        public static BinaryData FromFile(string path, string? mediaType);
        public static Task<BinaryData> FromFileAsync(string path, CancellationToken cancellationToken = default);
        public static Task<BinaryData> FromFileAsync(string path, string? mediaType, CancellationToken cancellationToken = default);
    }
  11. added
    api-approvedAPI was approved in API review, it can be implemented
    and removed
    api-ready-for-reviewAPI is ready for review, it is NOT ready for implementation
    on Aug 13, 2024
  12. added
    in-prThere is an active PR which will close this issue when it is merged
    on Sep 1, 2024
  13. modified the milestones: Future, 10.0.0 on Nov 11, 2024
  14. locked and limited conversation to collaborators on Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

api-approvedAPI was approved in API review, it can be implementedarea-System.Memoryin-prThere is an active PR which will close this issue when it is merged

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions