-
Notifications
You must be signed in to change notification settings - Fork 1k
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
docs: fix docker image names + remove monitoring interceptor docs #4076
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for the update!
@@ -114,7 +114,7 @@ Start the ksqlDB CLI: | |||
|
|||
```bash | |||
docker run --network tutorials_default --rm --interactive --tty \ | |||
confluentinc/cp-ksql-cli:{{ site.release }} \ | |||
confluentinc/ksqldb-cli:{{ site.release }} \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the ksql
command also needed here before the server endpoint? For example:
docker run --network tutorials_default --rm --interactive --tty \
confluentinc/ksqldb-cli:0.6.0 ksql http://ksqldb-server:8088
There was a problem hiding this 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
@@ -166,7 +166,7 @@ Load the Streaming Data to KSQL | |||
1. Launch the KSQL CLI: | |||
|
|||
```bash | |||
docker-compose exec ksql-cli ksql http://ksql-server:8088 | |||
docker-compose exec ksql-cli ksql http://ksqldb-server:8088 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be ksqldb-cli
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's correct, that's the server endpoint the CLI connects to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jim is correct, the service is named ksql-cli
in the docker-compose: https://github.com/confluentinc/examples/blob/5.3.1-post/clickstream/docker-compose.yml#L97 so that part is fine. However, ksqldb-server:8088
should be ksql-server:8088
so Mike has still found a bug! Will update accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, my bad. This example is using the CP Docker images, and the containers have the ksql
name instead of ksqldb
. Sorry for the confusion.
Description
Two docs changes:
cp-ksql-server
andcp-ksql-cli
docker images toksqldb-server
andksqldb-cli
instead, ascp-ksql-server:0.6.0
andcp-ksql-cli:0.6.0
don't existTesting done
Docs-only change.
Reviewer checklist