Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ClientModel: Release Azure.Variant in the Azure.Core package #32978

Open
4 of 5 tasks
heaths opened this issue Dec 8, 2022 · 7 comments
Open
4 of 5 tasks

ClientModel: Release Azure.Variant in the Azure.Core package #32978

heaths opened this issue Dec 8, 2022 · 7 comments
Assignees
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved.
Milestone

Comments

@heaths
Copy link
Member

heaths commented Dec 8, 2022

Feature Description

Azure.Variant is an implementation of a tagged union. It provides a type we can use for both primitives and value types to avoid boxing .NET intrinsic types. For example, in libraries like KeyVault mentioned below, it could be used to hold both a string and an int without boxing the int.

API Proposal

dotnet/runtime#28882

Additions we'd like to make

  • Improved UX for nulls
  • Improved debugger viewing
  • Allow reference type values, e.g. string, to return null from As<T> without throwing
  • Don't allow setting Variant to a Variant
    The following code should print "System.String", but it prints "Azure.Value":
    var value1 = new Value("hi");
    var value2 = new Value(value1);
    Console.WriteLine(value2.Type);
  • Support numeric conversions, e.g.:
    If a cast from S to D works in standard C# code, it should work when S is stored in Variant.
    var value2 = new Value(1);
    object int64 = value2.As<long>();

Libraries we expect to use it

  • JobRouter
  • Azure.Core
@heaths heaths added Client This issue points to a problem in the data-plane of the library. Azure.Core labels Dec 8, 2022
@heaths heaths added this to the 2023-01 milestone Dec 8, 2022
@christothes
Copy link
Member

Do we have any other context on the Azure.Value type?

@heaths
Copy link
Member Author

heaths commented Jan 5, 2023

@KrzysztofCwalina and @tg-msft asked me to consider using it for an upcoming KV release, but seems it won't make it in time. They asked me to open a tracking issue.

@jsquire jsquire modified the milestones: 2023-01, 2023-02 Jan 19, 2023
@jsquire jsquire modified the milestones: 2023-02, 2023-03 Feb 15, 2023
@jsquire jsquire modified the milestones: 2023-03, Backlog Mar 27, 2023
@annelo-msft annelo-msft added the feature-request This issue requires a new behavior in the product in order be resolved. label Jun 21, 2023
@annelo-msft
Copy link
Member

Per @KrzysztofCwalina:

The following package GA is blocked on this. We want to not ship this type and use Value instead: https://apiview.dev/Assemblies/Review/e1389ac1af984a86a079127e8093937a#Azure.Communication.JobRouter.LabelValue

@annelo-msft
Copy link
Member

annelo-msft commented Jun 23, 2023

Filling in some context:

@heaths
Copy link
Member Author

heaths commented Jun 23, 2023

KeyVaultSettingValue does do one thing a little differently than Value, and it may or may not be worth considering. To avoid polymorphic types for a single value, we recommended a type: string property for the value. Any primitive can encode as a string, so this avoided more complexity in the generated models we use as an HLC. Thus, ToString always returns the string-formatted value, and customers can set a string. This allows a customer on an older SDK version to read/write values that are added to the service, which are not part of the schema (the key/value pairs themselves).

Maybe Value is meant more for polymorphic types, but this might be something to consider. At least when I opened this tracking issue, Value did not support an everything-as-a-string fallback.

@annelo-msft
Copy link
Member

A service team has asked how they can indicate in a swagger or typespec file that Value should be used in a model - it would be good to provide a solution to this.

@annelo-msft annelo-msft changed the title Release Azure.Value in the Azure.Core package Release Azure.Variant in the Azure.Core package Sep 7, 2023
@jsquire jsquire modified the milestones: 2023-10, Backlog Oct 19, 2023
@annelo-msft annelo-msft modified the milestones: Backlog, 2024-04 Mar 12, 2024
@annelo-msft annelo-msft changed the title Release Azure.Variant in the Azure.Core package ClientModel: Release Azure.Variant in the Azure.Core package Mar 12, 2024
@annelo-msft
Copy link
Member

@KrzysztofCwalina, we should touch-base on whether or not this is currently a priority, and if so, what our approach is now given System.ClientModel

@jsquire jsquire modified the milestones: 2024-04, 2024-05 Apr 30, 2024
@jsquire jsquire modified the milestones: 2024-05, 2024-06 May 20, 2024
@jsquire jsquire modified the milestones: 2024-06, Backlog Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved.
Projects
None yet
Development

No branches or pull requests

7 participants