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

Influxdb connector #1680

Merged
merged 95 commits into from Jun 28, 2019

Conversation

@gkatzioura
Copy link
Contributor

gkatzioura commented May 5, 2019

Purpose

This is the Alpakka connector for InfluxDB

References

References #1054

Changes

Added the connector for InfluxDB. Added InfluxDB on docker compose configuration.

Background Context

Commits are based on the source code of existing connectors like Elasticsearch and OrientDB.

Sources are created through queries. Inserts can be done both through models using the InfluxDB mapper or through Points. Batch operations are possible and enabled.

@gkatzioura

This comment has been minimized.

Copy link
Contributor Author

gkatzioura commented May 5, 2019

I get a failure both on the compatibility check (it also happens on master). Any help on fixing it, it will be greatly appreciated

[error] (awslambda / mimaReportBinaryIssues) akka-stream-alpakka-awslambda: Binary compatibility check failed!
[error] (influxdb / mimaPreviousClassfiles) sbt.librarymanagement.ResolveException: unresolved dependency: com.lightbend.akka#akka-stream-alpakka-influxdb_2.12;1.0-M3: not found
[error] (dynamodb / mimaReportBinaryIssues) akka-stream-alpakka-dynamodb: Binary compatibility check failed!
[error] (reference / mimaPreviousClassfiles) sbt.librarymanagement.ResolveException: unresolved dependency: com.lightbend.akka#akka-stream-alpakka-reference_2.12;1.0-M3: not found
@ennru ennru added the p:new label May 6, 2019
@ennru

This comment has been minimized.

Copy link
Member

ennru commented May 6, 2019

Thank you for this PR.

You may switch off the MiMa check for the new module with .disablePlugins(MimaPlugin). The other problems you mention don't show for me or on Travis.

@ennru

This comment has been minimized.

Copy link
Member

ennru commented May 6, 2019

Please add the new module to .travis as well.

Copy link
Member

ennru left a comment

Great, I think it is better to keep it simple and focus on the most crucial use cases. This is almost there but needs documentation.

* INTERNAL API
*/
@InternalApi
private[influxdb] final class InfluxDbRawSourceStage(query: Query, influxDB: InfluxDB)

This comment has been minimized.

Copy link
@ennru

ennru Jun 14, 2019

Member

I believe InfluxDbSourceLogic and InfluxDbRawSourceStage should have a common base class as they share most of the code.

val messages = grab(in)
if (messages.nonEmpty) {

influxDB.enableBatch(BatchOptions.DEFAULTS)

This comment has been minimized.

Copy link
@ennru

ennru Jun 14, 2019

Member

Would it be important to make these settings configurable?

This comment has been minimized.

Copy link
@gkatzioura

gkatzioura Jun 14, 2019

Author Contributor

This was removed as explained here

influxDB.enableBatch(BatchOptions.DEFAULTS)
write(messages)
val writtenMessages = messages.map(m => new InfluxDbWriteResult(m, None))
influxDB.close()

This comment has been minimized.

Copy link
@ennru

ennru Jun 14, 2019

Member

This API call is a bit surprising, but that might be just the way it is. Does this replace the disableBatch call which the API docs of enableBatch require?

This comment has been minimized.

Copy link
@gkatzioura

gkatzioura Jun 14, 2019

Author Contributor

@ennru
Exactly. Once close() is called the batch is disabled and the messages are send to the database.
However this is no longer needed and It shall be removed.
I extracted the functionality, that the driver provided with batching enabled, and is currently executed by the connector itself.
This means that on a onPush call, the messages are written to the database in one http request.
Therefore the user has control over the batch size and the delay through the connector without the need to be aware of the driver internals.

gkatzioura added 4 commits Jun 14, 2019
…e() as the InfluxDB connector handles the batching operation by itself.
…st pull on InfluxDb connector
…ogic for InfluxDb connector
@ennru

This comment has been minimized.

Copy link
Member

ennru commented Jun 25, 2019

@gkatzioura This looks good now, I propose we'll merge it and mark it as "API may change" so we keep the door open to apply more of @adkafka's suggestions.
We require documentation, though.
It would be perfect to get this in before we release Alpakka 1.0.3 on 2019-07-03.

@ennru ennru added the p:influxdb label Jun 25, 2019
@gkatzioura

This comment has been minimized.

Copy link
Contributor Author

gkatzioura commented Jun 27, 2019

@ennru
Just added the documentation and also added the @ApiMayChange annotation.
The api will indeed change apart from extra recommendations due to a major release on InfluxDB 2.0. Is there any faq on handling and contributing to api changes?

gkatzioura added 7 commits Jun 27, 2019
@ennru

This comment has been minimized.

Copy link
Member

ennru commented Jun 28, 2019

Thank's for the heads-up about the new version. I'll put a not in the documentation about both "API may change" and the upcoming InfluxDB version.

@ennru ennru added this to the 1.1.0 milestone Jun 28, 2019
@ennru
ennru approved these changes Jun 28, 2019
Copy link
Member

ennru left a comment

LGTM.

@ennru ennru changed the title Influxdb alpakka connector Influxdb connector Jun 28, 2019
@ennru ennru merged commit cd12632 into akka:master Jun 28, 2019
1 of 2 checks passed
1 of 2 checks passed
Travis CI - Pull Request Build Failed
Details
typesafe-cla-validator All users have signed the CLA
Details
@ennru

This comment has been minimized.

Copy link
Member

ennru commented Jun 28, 2019

Thank you so much for this effort! We've seen the interest for InfluxDB support for a while, but didn't get to look into it. Highly appreciated.

@gkatzioura

This comment has been minimized.

Copy link
Contributor Author

gkatzioura commented Jun 28, 2019

@ennru
Thank you for merging!
I am keeping a close eye on the changes that occur. The version 2.0 is going to be huge, with extra functionality so I will be back with some contributions.

cheleb added a commit to cheleb/alpakka that referenced this pull request Jul 5, 2019
A new connector to stream data to and from InfluxDB.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.