Closed mikelaws closed 8 years ago
elasticsearch-release
commented
8 years ago Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.
mikelaws
commented
8 years ago Confirmed working with InfluxDB 0.10.0: able to write to sprintf-formatted database and measurement names, and via HTTP and HTTPS.
Also confirmed the question from @jordansissel on December 9th regarding nil values for @user and @password.value - InfluxDB handles the empty user/password values fine when authentication is not configured (allows anonymous access, as expected).
mikelaws
commented
8 years ago @suyograo, I didn't realize this project didn't have a maintainer (unless I'm mistaken). Any chance someone on the Elastic side can help nudge this through the process? I'm currently running these changes in prod, but would like to be able to point folks to something more official. Thanks!
suyograo
commented
8 years ago @mikelaws I just merged https://github.com/logstash-plugins/logstash-output-influxdb/pull/29. Does this need a rebase? Can you review please?
suyograo
commented
8 years ago @mikelaws Also, I had published 3.0.0 with #30. Can you bump this version?
mikelaws
commented
8 years ago @suyograo Thanks! Rebase done. Also bumped the version to 3.1.0 and updated the change log accordingly.
suyograo
commented
8 years ago LGTM, thanks
elasticsearch-bot
commented
8 years ago Suyog Rao merged this into the following branches!
| Branch | Commits |
|---|---|
| master | 0aae1feee9488942bc04556b3110a732c3b1e90c, c437bc15a82ebeb11ce2a7d7a215476153b0caf1 |
Sorry to muddy the InfluxDB 0.9 waters even more... This PR includes fixes to address SSL/TLS support (http and https URLs - #32), and sprintf formatting in the DB name (#31). This is based on a fork of the pr/29 branch (everything in pr #29 and addressing #24), with all of the great work from @contentfree and @timgriffiths (including your fixes for special character handling discussed in pr #30) merged in as well.
Tests have been added to validate SSL/TLS config handling, and proper handling/buffering/flushing of events destined for a single, and multiple databases (via sprintf formatting in the configuration). Also added a test that exercises sprintf formatting in the "measurement" name. These are in addition to @timgriffiths tests, which appear to provide good coverage for his special character handling fixes and run fine in my environment.
Signed the CLA about a week ago. Currently running against InfluxDB 0.9.6, but should work against any 0.9 release, and should also be forward-compatible with InfluxDB 0.10.x.
If we can get a quick thumbs-up on the latest code changes, perhaps we can get this plugin moving forward and close several PRs and issues in the process. I see that @jordansissel has reviewed some of the earlier code changes, and suggested the bump in major version, which makes sense to me given the breaking changes and new features/fixes.