feat: separate out enum names and values #9518
Conversation
5cf1dde to
79740f7
Compare
56d2a80 to
f71c45a
Compare
|
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
|
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
|
This PR was closed because it has been stalled for 7 days with no activity. |
|
I split some work out to #10058, so will rebase on that once merged. |
|
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
|
This PR was closed because it has been stalled for 7 days with no activity. |
| conv.register_unstructure_hook(enum.Enum, to_enum_name) | ||
| conv.register_structure_hook(enum.Enum, from_enum_name) | ||
|
|
||
| conv.register_unstructure_hook(base.Enum, to_enum_name) | ||
| conv.register_structure_hook(base.Enum, from_enum_name) | ||
|
|
||
| conv.register_unstructure_hook(Enum, to_enum_name) | ||
| conv.register_structure_hook(Enum, from_enum_name) |
There was a problem hiding this comment.
You can avoid duplication here by using your own predicate function:
| conv.register_unstructure_hook(enum.Enum, to_enum_name) | |
| conv.register_structure_hook(enum.Enum, from_enum_name) | |
| conv.register_unstructure_hook(base.Enum, to_enum_name) | |
| conv.register_structure_hook(base.Enum, from_enum_name) | |
| conv.register_unstructure_hook(Enum, to_enum_name) | |
| conv.register_structure_hook(Enum, from_enum_name) | |
| def is_enum(t) -> bool: | |
| return issubclass(t, enum.Enum) | |
| conv.register_unstructure_hook_func(is_enum, to_enum_name) | |
| conv.register_structure_hook_func(is_enum, from_enum_name) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Hm. So this doesn't work. a2fd403
https://dagger.cloud/jedevc/traces/26e7a81edf9304da7090245ef15893fe
I could be missing something obvious.
But I seem to remember having this conversation in discord here: https://discord.com/channels/707636530424053791/1337048414751559680/1341125495189798962
There was a problem hiding this comment.
I refactored a bit to allow for it, pushed to your branch.
Basically the dagger.client.base.Enum class extended str making members of the class basically strings. The problem is that cattrs would dispatch the hook for "str" when encountering a member, instead of the custom added. We could override the hook for "str" too but I want to simplify.
I want to deprecate dagger.Enum in favor of enum.Enum:
Still added a type hook (instead of function hook) if an enum decorated with @dagger.enum_type extends a primitive type like str or int for backwards compat or if users end up using it on their own directly.
So this should cover all enums in a simple way without having to register 3 different types and still missing cases (e.g., user extending str and enum.Enum without one of our base classes).
c2ddc82 to
56f07e5
Compare
Signed-off-by: Justin Chadwell <me@jedevc.com>
This allows enums: - to have associated values, as well as their pre-existing names - to have versioned members using dagql views Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
This can happen after weird string processing. Signed-off-by: Justin Chadwell <me@jedevc.com>
Enums are complicated. So handle them separately, instead of happily bundling them into primitive types. Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
I want to deprecate extending dagger.Enum in favor of stdlib's enum.Enum in a follow-up PR so removed str as a base class from dagger.client.base.Enum and adjusted tests to more explicitly return the value instead of the string representation. Additionallly motivated because in cattrs it bypasses the custom hook for enums, and using the one for primitive strings instead, making registration a bit more complicated. Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com>
Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com>
Update TS SDK to support enum/value feature introduced by dagger#9518. Handle serialization through metadata with an external function generated in client. Handle deserialization inside client gen with an function generated in client. Add tests for enum generation Signed-off-by: Tom Chauveau <tom@dagger.io>
Update TS SDK to support enum/value feature introduced by dagger#9518. Handle serialization through metadata with an external function generated in client. Handle deserialization inside client gen with an function generated in client. Add tests for enum generation Signed-off-by: Tom Chauveau <tom@dagger.io>
Update TS SDK to support enum/value feature introduced by dagger#9518. Handle serialization through metadata with an external function generated in client. Handle deserialization inside client gen with an function generated in client. Add tests for enum generation Signed-off-by: Tom Chauveau <tom@dagger.io>
Update TS SDK to support enum/value feature introduced by dagger#9518. Handle serialization through metadata with an external function generated in client. Handle deserialization inside client gen with an function generated in client. Add tests for enum generation Signed-off-by: Tom Chauveau <tom@dagger.io>
Update TS SDK to support enum/value feature introduced by dagger#9518. Handle serialization through metadata with an external function generated in client. Handle deserialization inside client gen with an function generated in client. Add tests for enum generation Signed-off-by: Tom Chauveau <tom@dagger.io>
Fixes #8671 - the aim is to fix all the remaining little nits with enums that make them tricky to use (also wanting to unblock #8608).
Warning
👷♀️ Depends on #10566.
TODO:
withEnumMemberAPI for enum memberswithEnumValuetowithEnumMemberin all SDKsTypescript(skipping for now, turns out this is just really hard to do, since the typescript enums aren't richly typed).Handle renamed types,(deferred into Module version compat should handle type changes #10103)ImageMediaType->ImageMediaTypes,EnumValueTypeDef->EnumMemberTypeDefEnumTypeDef.values