feat(Docker): Add support Avro

qubole / streamx

kafka-connect-s3 : Ingest data from Kafka to Object Stores(s3)

Apache License 2.0

97 stars 54 forks source link

feat(Docker): Add support Avro #27

Closed jocelyndrean closed 7 years ago

jocelyndrean commented 7 years ago

What's this PR purpose?

Support Avro with Docker

add a new file "/config/connect-avro-distributed.properties"
add a boolean var CONNECT_USE_AVRO to switch from "connect-distributed.properties" to "connect-avro-distributed.properties" So no need to extend that to add new properties if it does not exist.
Update readme file to add small documentation about Docker with streamx

Example on how to use it:

docker run -d -p 8083:8083 --env CONNECT_BOOTSTRAP_SERVERS=public_dns:9092 --env CONNECT_AWS_ACCESS_KEY=youracesskey --env CONNECT_AWS_SECRET_KEY=yoursecretkey  --env CONNECT_KEY_CONVERTER=io.confluent.connect.avro.AvroConverter --env CONNECT_VALUE_CONVERTER=io.confluent.connect.avro.AvroConverter --env CONNECT_KEY_CONVERTER_SCHEMA_REGISTRY_URL=http://your.schema.registry.com:8081 --env CONNECT_VALUE_CONVERTER_SCHEMA_REGISTRY_URL=http://your.schema.registry.com:8081 qubole/streamx

PraveenSeluka commented 7 years ago

Hi @jocelyndrean

Thanks for submitting this PR.

I think, we can simplify it by just reusing the connect-distributed.properties file and add "key.converter.schema.registry.url" and "value.converter.schema.registry.url" properties (we dont need two connect configs, just 1 is enough)
Users could set CONNECT_KEY_CONVERTER and CONNECT_VALUE_CONVERTER to use Avro Converter. So do we need CONNECT_USE_AVRO. Yes, it makes it simpler as it becomes 1 property instead of 2. But user have to learn this property exists. Also, the rest is consistent - for any override the convention is CONNECT_property

Let me know what you think about this. I think, we can skip CONNECT_USE_AVRO and be consistent.

ReadMe looks good.

jocelyndrean commented 7 years ago

Hi @PraveenSeluka, thanks for your feedback ! At first, I didn't want to alter the default configuration files "connect-avro-distributed.properties" and "connect-distributed.properties" from Confluent (to stay with something very close to the original files). But, that doesn't really make sense, and we already add the property "rest.port". Let me fix that and I ping you when it's done :)

jocelyndrean commented 7 years ago

Hi @PraveenSeluka : fixed ! I removed connect-avro-distributed.properties file and add

key.converter.schema.registry.url=http://localhost:8081
value.converter.schema.registry.url=http://localhost:8081 to connect-distributed.properties I also removed the env var CONNECT_USE_AVRO, this was useless.

PraveenSeluka commented 7 years ago

Thanks @jocelyndrean for changes. It looks good, I will do one more pass by end of day.