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

Deploy multiple #32

Merged
merged 10 commits into from
Jan 30, 2019
Merged

Deploy multiple #32

merged 10 commits into from
Jan 30, 2019

Conversation

FrancoisPoinsot
Copy link
Contributor

When there is too many connector to deploy, the distributed kafka-connect workers may have problem to handle the load.

Currently the cli only handle them one by one, so we resorted to to it in parallel in the bash itself.
But error handling and throttling it in bash is painful.
So why not do that in the cli.

Changes:

  • In lib, add feature DeployMultipleConnector with a max parallel deployment fixed to 3.
    Multiple error will be aggregated and returned once all connectors configs given have been tried.

  • Rework a bit the add the first unit tests. They are still not yet automatically executed.
    Mostly isolated the low level client which only implement the kafka-connect API from the higher level client which expose all the custom feature we use. Such as DeployMultipleConnector.

  • In cli make the --file argument support referencing a folder.
    cli will try to parse every files in this folder. No subfolder will be searched.
    Goal is to reference a folder full of config to deploy and forget the parallelism/error handling on the bash side.

cli/cmd/deploy.go Outdated Show resolved Hide resolved
lib/Makefile Outdated Show resolved Hide resolved
@FrancoisPoinsot
Copy link
Contributor Author

every comment has been either answered or applied
please review

cli/cmd/create.go Outdated Show resolved Hide resolved
Copy link

@fsilberstein fsilberstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good! Great job 👍
When you create the release, maybe use a major version because of the breaking change for the flag name

@FrancoisPoinsot FrancoisPoinsot merged commit 96d47e6 into master Jan 30, 2019
@FrancoisPoinsot FrancoisPoinsot deleted the deploy-multiple branch January 30, 2019 09:27
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.

2 participants