-
Notifications
You must be signed in to change notification settings - Fork 128
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
add escaping for backslashes, fixes #718 #720
base: master
Are you sure you want to change the base?
add escaping for backslashes, fixes #718 #720
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. |
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.
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 test
completes successfully