rfc: type metadata#1855
Conversation
|
cc @eapolinario / @EngHabu. This looks pretty cool to me, just that the metadata field may be overloaded, especially in the dataclass case? |
d182642 to
4fb965e
Compare
EngHabu
left a comment
EngHabu
left a comment
There was a problem hiding this comment.
Awesome! I've a few questions if you can please take a look
| ], | ||
| ``` | ||
|
|
||
| ## 3 Proposed Implementation |
There was a problem hiding this comment.
Can we include the work here to strip them in here.
There was a problem hiding this comment.
can you help me understand why?
There was a problem hiding this comment.
this is so that the backend representation at runtime does not bloat and is important to remain efficient and performant
EngHabu
left a comment
EngHabu
left a comment
There was a problem hiding this comment.
Awesome! one comment (to introduce a new field/message) and then let's merge it!
| Users of flyte wish to have access to arbitrary parameter metadata specified in | ||
| typing annotations. | ||
|
|
||
| The `LiteralType` definition in flyteidl already has a `metadata` field in its |
There was a problem hiding this comment.
Per the discussion in the comments here and on chat, I think it's better to introduce a new message and a corresponding field (and potentially deprecating metadata altogether eventually)...
And the rest of the spec look great.
message TypeMetadata {
google.protobuf.Struct annotations = 1;
// string type_hint = 2;
// google.protobuf.Struct json_schema = 2;
...
}
message LiteralType
{
...
TypeMetadata metadata = 6; // For now I guess we have to call it type_metadata
}There was a problem hiding this comment.
what purpose does replacing
google.protobuf.Struct metadata = 6;
with a nested message serve. Its just one more level of unwrapping for the exact same information?
Also, doesn't adding an additional field make more sense.?We don't break support for existing use of metadata for flyte specific implementation but allows for use of annotations as intended in a way that doesn't conflict.
Signed-off-by: Kenny Workman <kennyworkman@sbcglobal.net>
Signed-off-by: Kenny Workman <kennyworkman@sbcglobal.net>
Signed-off-by: Kenny Workman <kennyworkman@sbcglobal.net>
Signed-off-by: Kenny Workman <kennyworkman@sbcglobal.net>
2d963c2 to
38e1feb
Compare
3 participants
No description provided.