-
Notifications
You must be signed in to change notification settings - Fork 129
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
feat: add FluxRecord.getRow() with response data stored in List #439
Conversation
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.
Thanks for your PR 👍
There are a few requirements that must be be satisfy before we accept the PR:
Codecov ReportBase: 88.38% // Head: 88.39% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #439 +/- ##
============================================
+ Coverage 88.38% 88.39% +0.01%
Complexity 776 776
============================================
Files 172 172
Lines 6999 7008 +9
Branches 377 378 +1
============================================
+ Hits 6186 6195 +9
Misses 693 693
Partials 120 120
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
client-core/src/main/java/com/influxdb/query/internal/FluxCsvParser.java
Outdated
Show resolved
Hide resolved
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.
LGTM 🌹
Proposed Changes
Adding possibility of accessing response data in List FluxRecord.getRow().
In case of using pivot on data, where field contains labels that occur by default in the annotated CSV (f.e. "result" or "table"), could be duplicated column names in response. In that case FluxRecord.getValues() (Map), which can hold only unique keys, doesn't show complete data. This edge case is solved by using FluxRecord.getRow() (List).
Checklist
mvn test
completes successfully