Skip to content
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

Unify Point's and InfluxDBMapper's POJO saving logic #979

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

eranl
Copy link
Contributor

@eranl eranl commented Dec 5, 2023

Currently, there are two duplicate and somewhat divergent versions of this logic:

  • Point doesn't check that values conform to field types and doesn't constrain allowed value types
  • InfluxDBMapper uses a time field rather than @TimeColumn

This change unifies the logic and takes the stricter path, which means that it may break existing code that relies on the current lenient logic.

@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (d697143) 61.35% compared to head (abbdf75) 60.98%.

❗ Current head abbdf75 differs from pull request most recent head 950708f. Consider uploading reports for the commit 950708f to get more accurate results

Files Patch % Lines
src/main/java/org/influxdb/dto/Point.java 78.94% 1 Missing and 3 partials ⚠️
...rc/main/java/org/influxdb/impl/InfluxDBMapper.java 75.00% 1 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #979      +/-   ##
============================================
- Coverage     61.35%   60.98%   -0.38%     
+ Complexity      450      435      -15     
============================================
  Files            70       70              
  Lines          2593     2568      -25     
  Branches        278      276       -2     
============================================
- Hits           1591     1566      -25     
  Misses          932      932              
  Partials         70       70              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@majst01 majst01 left a comment

Choose a reason for hiding this comment

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

Nice, thanks

@majst01 majst01 merged commit e93949f into influxdata:master Dec 5, 2023
18 checks passed
@eranl eranl deleted the unify-save branch December 5, 2023 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants