Skip to content

Conversation

mmacai
Copy link
Collaborator

@mmacai mmacai commented Sep 18, 2019

Description

  • updated documentation
  • added test cases
  • added changelog entry
  • added option to set topic and schema for Kafka and topic for Kinesis via Proxy config
  • deprecated old way using env vars
  • changed partition key to be optional and randomly generated if not present
  • added kinesis + localstack readme

Closes #229

What to look out for

@kevinbader - want to go out firstly with documentation and thoughts and then start with implementation. Please check notes below.

  • check upcoming changes in docs/config
  • Kafka should be covered with integration tests
  • Kinesis can be tested using the instructions in examples/kinesis-localstack/README.md

Notes:

  • we keep (but deprecate) existing way of configuring the Kafka/Kinesis endpoints in reverse proxy -- fallback method
  • add new fields to configure each endpoint separately with the topic and schema fields - perhaps they should be named request_topic and request_schema to make it more clear, that they are used just for publishing, not consuming
  • request body -- target_partition would be optional and if not set -- randomized
  • current version would firstly check the topic and schema fields in the config, if not present -> check env vars
    • version 3.0 would remove second part and make the topic field mandatory

Extra notes:

  • it's done in a copy/paste way (handle_http_request function) since the deprecated way will be removed eventually and therefore extracting common logic didn't make sense to me

@mmacai mmacai changed the title WIP: 229 - Reverse proxy - new Kafka/Kinesis config 229 - Reverse proxy - new Kafka/Kinesis config Oct 31, 2019
@mmacai mmacai requested a review from kevinbader October 31, 2019 15:53
@kevinbader
Copy link
Contributor

@mmacai I can't seem to run the i9n tests, not sure if your changes are the cause of it though:

$ ./integration_tests/kafka_tests/run.sh
Creating network "kafka_tests_default" with the default driver
Creating zookeeper ... done
Creating kafka     ... error

ERROR: for kafka  Cannot start service kafka: b'Ports are not available: /forwards/expose/port returned unexpected status: 500'

ERROR: for kafka  Cannot start service kafka: b'Ports are not available: /forwards/expose/port returned unexpected status: 500'
ERROR: Encountered errors while bringing up the project.

@mmacai
Copy link
Collaborator Author

mmacai commented Nov 6, 2019

@mmacai I can't seem to run the i9n tests, not sure if your changes are the cause of it though:

$ ./integration_tests/kafka_tests/run.sh
Creating network "kafka_tests_default" with the default driver
Creating zookeeper ... done
Creating kafka     ... error

ERROR: for kafka  Cannot start service kafka: b'Ports are not available: /forwards/expose/port returned unexpected status: 500'

ERROR: for kafka  Cannot start service kafka: b'Ports are not available: /forwards/expose/port returned unexpected status: 500'
ERROR: Encountered errors while bringing up the project.

Can you try to check the ports (e.g. by sudo lsof -i :17092)? This seems like a docker issue. Tried again on my machine and no problem.

kevinbader
kevinbader previously approved these changes Dec 13, 2019
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.

Multiple Kafka topics for publish events
2 participants