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

Add ability to specify the schema registry url to the CLI in local mode. #545

Merged

Conversation

apurvam
Copy link
Contributor

@apurvam apurvam commented Dec 19, 2017

With this patch, we can use schema registry in the quickstart docker images. We would need to update the https://github.com/confluentinc/ksql/blob/master/docs/quickstart/quickstart-docker.md so that we update the exec command to:

docker-compose exec ksql-cli ksql-cli local --bootstrap-server kafka:29092 --schema-registry-url schema-registry:8081

…Will enable us to use avro in the quickstart
@apurvam apurvam changed the base branch from 4.0.x to master December 19, 2017 01:34
@apurvam apurvam changed the title Add schema registry opt to local cli Add ability to specify the schema registry url to the CLI in local mode. Dec 19, 2017
@apurvam
Copy link
Contributor Author

apurvam commented Dec 19, 2017

retest this please

@apurvam
Copy link
Contributor Author

apurvam commented Dec 19, 2017

The same CLITest failed twice. It is getting less stable now apparently :(

Copy link
Contributor

@hjafarpour hjafarpour left a comment

Choose a reason for hiding this comment

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

One minor comment, otherwise, LGTM.

+ "schema registry which the KSQL instance can connect to."
)
String schemaRegistryUrl;

Copy link
Contributor

Choose a reason for hiding this comment

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

Also mention that setting the schema registry property here will overwrite the one in the properties file.

Copy link
Contributor

@rodesai rodesai left a comment

Choose a reason for hiding this comment

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

LGTM

@apurvam apurvam merged commit acce231 into confluentinc:master Dec 19, 2017
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.

3 participants