add escaping for backslashes, fixes #718#720
add escaping for backslashes, fixes #718#720oliverrahner wants to merge 1 commit intoinfluxdata:masterfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #720 +/- ##
============================================
+ Coverage 88.37% 88.39% +0.01%
+ Complexity 786 784 -2
============================================
Files 174 174
Lines 7091 7093 +2
Branches 391 391
============================================
+ Hits 6267 6270 +3
+ Misses 698 697 -1
Partials 126 126 ☔ View full report in Codecov by Sentry. |
bednar
left a comment
There was a problem hiding this comment.
Thank you so much for your Pull Request!
To ensure that we maintain the quality of our client and to prevent potential regressions in the future, could you please add your example from this issue comment into our integration tests? Specifically, we would appreciate it if you could incorporate it into com.influxdb.client.ITWriteApiBlocking.
This will not only help in verifying the current functionality but also in maintaining robustness through future updates.
Looking forward to your update, and thanks again for your valuable contribution!
Best regards.
|
Good thing you asked about the integration tests... turns out my fix doesn't work :-/ |
|
So, this probably still is true: Not fully aware of this library's policy, should it try to inform the user about the actual cause of the issue, such as a tag value ending in a backslash? Or should this be left to the Influx daemon (which doesn't do a good job explaining the issue in this case)? Independent from this question, the actual change introduced with this PR is probably still a good idea. |
Closes #718
Proposed Changes
Backslashes should also be escaped.
This is generally optional in InfluxDB, but in special cases such as the end of a tag value, it might become crucial
Checklist
mvn testcompletes successfully