Skip to content

feat: Add support for deserialization of POCO column property types with a "Parse" method, such as Guid #246

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Oct 8, 2021

Conversation

vesuvian
Copy link
Contributor

@vesuvian vesuvian commented Oct 5, 2021

Proposed Changes

Adds support to the FluxResultMapper to deserialize any column where the type exposes a static Parse method. This enables the use of POCO properties for many common data types, including Guid.

These types were already supported for serialization since most values are being ToString()'ed and written to influx.

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • dotnet test completes successfully
  • Commit messages are in semantic format
  • Sign CLA (if not already signed)

@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2021

Codecov Report

Merging #246 (8f04c1d) into master (1d5db86) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #246      +/-   ##
==========================================
+ Coverage   85.49%   85.56%   +0.07%     
==========================================
  Files          72       72              
  Lines        6424     6456      +32     
==========================================
+ Hits         5492     5524      +32     
  Misses        932      932              
Impacted Files Coverage Δ
Client.Core/Flux/Internal/FluxResultMapper.cs 75.36% <100.00%> (+7.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d5db86...8f04c1d. Read the comment docs.

Copy link
Contributor

@bednar bednar left a 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 few requirements that must be be satisfy before we accept the PR:

@vesuvian
Copy link
Contributor Author

vesuvian commented Oct 7, 2021

Alright, that's all fixed up. The "static" thing is a total force of habit. Please let me know if you need more changes!

Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bednar bednar merged commit f88e5bb into influxdata:master Oct 8, 2021
@bednar bednar added the enhancement New feature or request label Oct 8, 2021
@bednar bednar added this to the 3.1.0 milestone Oct 8, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants