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

Feature/influxdb2 #879

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

athornton
Copy link

I'm trying to create another connector for talking to InfluxDB v2 databases.

The fundamental idea is to copy the influxdb v1 connector, replace the Influx client library with a v2-appropriate one, and then fix up the various pieces.

Alas, I think I'm about as far as I can take it myself, never having written Scala, and my Java experience being from the late 1990s.

The fixups that seem to have worked: database becomes bucket, user/password become organization/token, client.write.Point no longer has a builder method but seems to have equivalent methods in the Point class.

The thing that didn't work: there's really no need for a BatchPoints class anymore, because we really want to just turn the points into java.util.List[Point] and then write them all at once with the writePoints() API. But there's a lot of ceremony around the BatchPoints and Builder class that I didn't know how to defang, and that has left me with my one remaining compilation error that I don't know what to do with:

[error] /home/adam/stream-reactor/kafka-connect-influxdb2/src/main/scala/com/datamountaineer/streamreactor/connect/influx/writers/InfluxBatchPointsBuilder.scala:134:21: method setPoints in class BatchPoints cannot be accessed as a member of com.datamountaineer.streamreactor.connect.influx2.javadto.BatchPoints from class InfluxBatchPointsBuilder in package writers
[error]         batchPoints.setPoints(points)

It looks to me like it should be able to find the method, but I'm obviously missing something.

Then there are also six type-coercion implicit-widening warnings-promoted-to-errors that I don't know (because I don't speak Scala) how to signal to the compiler "yes, I meant to do this", but they're all uses of Point.addField() and the underlying client will quite happily accept multiple types for field values.

So I could use some help getting this thing's compilation over the finish line and then I can start testing it against influxDB V2.

afausti and others added 6 commits September 8, 2022 10:35
- For double timestamp fields, increase time precision from milliseconds to microseconds
- It is now coerced to Long with microseconds precision
@athornton athornton marked this pull request as draft September 13, 2022 16:49
@davidsloan
Copy link
Collaborator

davidsloan commented Oct 24, 2022

Hi @athornton , thanks for your work on this. I'll try and help if I can.

I've just made a commit here taking your work and putting a little effort in to ensure it compiles:
57250e9

It didn't take me too long as it looks like you'd covered most of the work. Also I rebased and ensured conflicts were resolved. My branch is here: #886

At some point I will do some tidying up but I hope this unblocks your work for now!

In general it looks good, I think instead of having this along side the original connector, this can be a replacement. We don't need to keep 2 versions. Also, while working on this I noticed that the client supports connection to influx 1.8 so it is not necessary to maintain both.

@athornton
Copy link
Author

Huge thanks to @davidsloan ! My time right now is pretty overcommitted, but I will get back to this as I can. Looking forward to giving it a trial run.

@davidsloan
Copy link
Collaborator

Hi @athornton , just wondering how you're getting on with this one and if you need any further help to finish it off?

@athornton
Copy link
Author

I'm sorry; I haven't had a chance to work on it at all. If you'd like to take it and push it over the finish line you certainly have my blessing to do so.

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.

None yet

3 participants