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

feat: add blocking API to write a batch of records #296

Closed
wants to merge 1 commit into from

Conversation

pabigot
Copy link
Contributor

@pabigot pabigot commented Jan 1, 2022

Closes #295

Proposed Changes

This extends the blocking API to allow writing line protocol records that have already been combined into a string with newline separators.

Checklist

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

@codecov-commenter
Copy link

codecov-commenter commented Jan 1, 2022

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 91.83%. Comparing base (4218b50) to head (a575f20).
Report is 172 commits behind head on master.

Files with missing lines Patch % Lines
api/writeAPIBlocking.go 75.00% 1 Missing ⚠️

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #296      +/-   ##
==========================================
- Coverage   91.87%   91.83%   -0.04%     
==========================================
  Files          23       23              
  Lines        2042     2046       +4     
==========================================
+ Hits         1876     1879       +3     
- Misses        110      111       +1     
  Partials       56       56              

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

@vlastahajek
Copy link
Contributor

Thanks for the PR. The new function to write a single string is redundant. The WriteRecord function can also take just a single string.
It makes sense to improve this function to skip the loop and also to skip creating a string buffer if there is just a single line.

@pabigot
Copy link
Contributor Author

pabigot commented Jan 3, 2022

OK, I can create a new PR that implements that behavior change, but it's more complicated than it first sounds, and possibly introduces a performance hit by having to check for the conditions under which newlines must be inserted/appended. New API really seems more efficient and less confusing.

@vlastahajek
Copy link
Contributor

A single line doesn't need to have new-line-char. For such a simple case, breaking the API with a new feature seems to be too much.

@vlastahajek vlastahajek closed this Jan 4, 2022
@pabigot pabigot deleted the pabigot/20220101a branch January 4, 2022 15:33
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.

Add WriteBatch to WriteAPIBlocking
3 participants