-
Notifications
You must be signed in to change notification settings - Fork 702
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
Override literal in the case of attribute access of primitive values #6194
Conversation
Signed-off-by: Eduardo Apolinario <[email protected]>
Code Review Agent Run #31d7d8Actionable Suggestions - 1
Review Details
|
Changelist by BitoThis pull request implements the following key changes.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6194 +/- ##
==========================================
+ Coverage 36.86% 36.88% +0.01%
==========================================
Files 1318 1318
Lines 134773 134820 +47
==========================================
+ Hits 49683 49726 +43
- Misses 80758 80760 +2
- Partials 4332 4334 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
…tive-types Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Code Review Agent Run Status
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only question is around the uints... is it worth checking the value for overload?
Signed-off-by: Eduardo Apolinario <[email protected]>
Code Review Agent Run Status
|
Tracking issue
N/A
Why are the changes needed?
flyteconsole has a tough time rendering primitive attribute fields. For example, an execution of this workflow:
All the fields are misrendered, for example, the argument to

t_str
is rendered like:After this change this is rendered like:

What changes were proposed in this pull request?
In order to help flyteconsole render primitive attributes we turn those fields into scalar literals.
How was this patch tested?
unit tests and local sandbox.
Labels
Please add one or more of the following labels to categorize your PR:
This is important to improve the readability of release notes.
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link
Summary by Bito
This PR enhances the attribute path resolver by adding support for handling primitive type values in binary IDL. The changes introduce automatic conversion of primitive types (string, int, float64, and boolean) into their corresponding literal scalar representations, improving the system's flexibility in attribute handling.Unit tests added: False
Estimated effort to review (1-5, lower is better): 1