DOCS-127 - Add KSQL doc to CP #472

joel-hamill · 2017-11-20T20:23:51Z

This PR was originally filed/reviewed here, and then ported over to this repo (we decided against hosting directly on confluentinc/docs)

ewencp

found a couple of issues.

for the old content, do we actually want to hold onto it this way or just rely on the git history?

ewencp · 2017-11-20T22:28:13Z

docs/index.rst

+   ksql-clickstream-demo/index
+   examples
+   faq
+   contributing


this doesn't seem to exist?

ah, yeah. it exists as contributing.md - and should be displayed only in the GH repo.

ewencp · 2017-11-20T22:31:46Z

docs/syntax-reference.rst

+Example:
+
+.. code:: sql
+    SELECT * FROM pageviews


this needs a newline between the .. code command and its contents. currently it isn't rendering.

ewencp · 2017-11-20T22:32:03Z

docs/syntax-reference.rst

+**Tip:** If you want to select older data, you can configure KSQL to query the stream from the beginning.  You must
+run this configuration before running the query:
+
+.. code:: sql


same issue with newline here

ewencp

I didn't do a deep review of the content, but build structure and based on a quick scan of the rendered docs and build warnings, this LGTM. Should probably have one of the @confluentinc/ksql folks do the longer content/formatting review.

miguno

Made a quick first pass. Would appreciate reviews from other team members, though.

miguno · 2017-11-21T08:22:10Z

docs/ksql-clickstream-demo/Dockerfile

+
+ARG DOCKER_REGISTRY
+
+FROM ${DOCKER_REGISTRY}confluentinc/docker-demo-base:3.3.0


The version string 3.3.0 is outdated I think? cc @bluemonk3y

It should probably be 3.3.1
4.0.x is not available

changed to demo-base:3.3.1

miguno · 2017-11-21T08:23:30Z

docs/ksql-clickstream-demo/Dockerfile

+ADD demo/*sh /usr/share/doc/ksql-clickstream-demo/
+ADD demo/*sql /usr/share/doc/ksql-clickstream-demo/
+ADD demo/*json /usr/share/doc/ksql-clickstream-demo/
+ADD demo/connect-config/null-filter-4.0.0-SNAPSHOT.jar /share/java/kafka-connect-elasticsearch/


Are we sure about the hardcoded 4.0.0-SNAPSHOT version string? Shouldn't this be sth like ${KSQL_VERSION}? (I don't know, I just don't like hardcoding versions unless it's truly what we want here)

Also, should this jar file go to /share/java/kafka-connect-elasticsearch/ or to /usr/share/java/kafka-connect-elasticsearch/?

for now - its stuck on version 4.0.0-snapshot
correct dest is: /share/java/kafka-connect-elasticsearch/

miguno · 2017-11-21T08:24:48Z

docs/ksql-clickstream-demo/non-docker-clickstream.rst

+Prerequisites
+-------------
+
+-  :ref:`Confluent 3.3.0 <installation>` locally


Still 3.3.0?

that can be bumped to 4.0.0

miguno · 2017-11-21T08:25:10Z

docs/ksql-clickstream-demo/non-docker-clickstream.rst

+3.  Copy the Kafka Connect Elasticsearch configuration file
+    (``ksql/ksql-clickstream-demo/demo/connect-config/null-filter-4.0.0-SNAPSHOT.jar``)
+    to your Confluent installation ``share`` directory
+    (``confluent-3.3.0/share/java/kafka-connect-elasticsearch/``).


miguno · 2017-11-21T08:25:20Z

docs/ksql-clickstream-demo/non-docker-clickstream.rst

+
+    .. code:: bash
+
+        cp ksql-clickstream-demo/demo/connect-config/null-filter-4.0.0-SNAPSHOT.jar <path-to-confluent-3.3.0>/share/java/kafka-connect-elasticsearch/


3.3.0? 4.0.0-SNAPSHOT?

miguno · 2017-11-21T08:34:43Z

docs/quickstart/index.rst

+If you are running the Confluent Platform, you can stop it with this
+command.
+
+::


See my comment on bash highlighting above.

miguno · 2017-11-21T08:36:39Z

docs/quickstart/quickstart-non-docker.rst

+    :depth: 1
+
+Prerequisites
+    - :ref:`Confluent Platform 3.3.0 <installation>` or later is installed. This installation includes a Kafka broker, ZooKeeper, Schema Registry, REST Proxy, and Kafka Connect.


miguno · 2017-11-21T08:37:36Z

docs/quickstart/quickstart-non-docker.rst

+====================
+
+This topic describes how to setup a Kafka cluster and start KSQL in your local environment. After you complete these steps,
+you can return to the :ref:`create-a-stream-and-table` and start querying the data in the Kafka cluster.


Perhaps add a link to the KSQL Docker instructions? Think: "We also describe how to do this with Docker if you prefer that." Just in case people don't follow the top-level navigation down to here, but find this page via Google Search.

Same backlink for the Docker instructions to this page (think: "If you don't want to use Docker, we also have ...")

this doc appears in a table of contents and will show docker as well, a few inches to the left. i think it's best not to add extraneous (repeat) information.

miguno · 2017-11-21T08:38:08Z

docs/quickstart/quickstart-non-docker.rst

+Start Kafka
+-----------
+
+Navigate to the ``confluent-3.3.0`` directory and start the Confluent


miguno · 2017-11-21T08:39:01Z

docs/quickstart/quickstart-non-docker.rst

+
+.. code:: bash
+
+    $ ./bin/confluent start


We're not consistent IMHO. Sometimes we say ./bin/confluent start, sometimes we say <path-to-confluent-3.3.0>/bin/confluent start. Can we pick one and stick with it? Note that this should be consistent across the docs of all CP components.

should be <path-to-confluent>/bin/confluent start.

hjafarpour · 2017-11-21T21:08:01Z

docs/concepts.rst

+sent $100 to Bob, then Charlie sent $50 to Bob”. Facts in a stream are
+immutable, which means new facts can be inserted to a stream, but
+existing facts can never be updated or deleted. Streams can be created
+from a Kafka topic or derived from existing streams and tables. In both


Streams cannot be created from tables.

@hjafarpour so should this portion be deleted or derived from existing streams and tables?

It can be derived from existing streams.

hjafarpour · 2017-11-21T21:19:10Z

docs/config-ksql.rst

+    $ ksql-cli local --properties-file ./ksql.properties
+
+    # Start a KSQL server node (for client-server mode) with the custom properties above
+    $ ksql-server-start --properties-file ./ksql.properties


No --properties-file flag for ksql-server.

hjafarpour · 2017-11-21T21:21:44Z

docs/examples.rst

+       interests array<VARCHAR>, \
+       contact_info map<VARCHAR, VARCHAR>) \
+      WITH (kafka_topic='users-topic', \
+            value_format='JSON');


please add key = 'userid' to the WITH section similar to the create stream example.

bluemonk3y

once version changes are made - lgtm

joel-hamill · 2017-11-27T18:36:02Z

@bluemonk3y are we recommending 3.3.0 for quickstart (#472 (comment)) and 4.0.0 for clickstream (#472 (comment))? seems like we should pick one.

hjafarpour · 2017-11-28T16:59:53Z

docs/ksql-clickstream-demo/docker-clickstream.rst

+           Copyright 2017 Confluent Inc.                         
+
+           CLI v0.1, Server v0.1 located at http://localhost:9098
+


Version for CLI and server is 0.2 now.

changed globally

hjafarpour · 2017-11-28T17:00:50Z

docs/ksql-clickstream-demo/non-docker-clickstream.rst

+        Copyright 2017 Confluent Inc.                         
+
+        CLI v0.1, Server v0.1 located at http://localhost:9098
+


Same here, version is 0.2 now.

hjafarpour · 2017-11-28T17:01:33Z

docs/quickstart/index.rst

+                          =   Streaming SQL Engine for Kafka   =
+       Copyright 2017 Confluent Inc.                         
+
+       CLI v0.1, Server v0.1 located at http://localhost:9098


Version is 0.2 now.

miguno

Some comments, thanks @joel-hamill.

miguno · 2017-11-29T09:00:30Z

docs/README.md

@@ -1,7 +1,6 @@
 # KSQL Documentation

-| Overview |[Quick Start](/docs/quickstart#quick-start) | [Concepts](/docs/concepts.md#concepts) | [Syntax Reference](/docs/syntax-reference.md#syntax-reference) |[Demo](/ksql-clickstream-demo#clickstream-analysis) | [Examples](/docs/examples.md#examples) | [FAQ](/docs/faq.md#frequently-asked-questions)  | [Roadmap](/docs/roadmap.md#roadmap) | 
-|---|----|-----|----|----|----|----|----|
+The full Confluent Platform documentation is available at [docs.confluent.io](https://docs.confluent.io/current/ksql/docs/index.html).


It should be clear that this is pointing to the KSQL docs in CP.

I'd suggest sth like:

See the KSQL documentation in Confluent Platform.

miguno · 2017-11-29T09:01:02Z

docs/README.md

@@ -26,7 +25,41 @@ You can use KSQL in standalone, client-server, application, and embedded modes.

 ## Getting Started

-* Beginners: Try the [interactive quick start](/docs/quickstart#quick-start). The quick start configures a single instance in a lightweight Docker container or in a Kafka cluster. It demonstrates a simple workflow using KSQL to write streaming queries against data in Kafka.
-* Advanced users: Try the [end-to-end Clickstream Analysis demo](/ksql-clickstream-demo#clickstream-analysis).
+* Beginners: Try the [interactive quick start](quickstart/index.rst). The quick start configures a single instance in a lightweight Docker container or in a Kafka cluster. It demonstrates a simple workflow using KSQL to write streaming queries against data in Kafka.


FYI: I didn't have the time to double-check links etc.

miguno · 2017-11-29T09:07:42Z

docs/README.md

@@ -1,7 +1,6 @@
 # KSQL Documentation


Meta comment: There's a lot of duplicated content for README.md and docs/README.md. I'd suggest that we focus on the top-level README.md. The docs/README.md doesn't need (essentially) duplicate sections on "KSQL overview", "Use cases", "Modes of operaiton", "Getting started", etc.

My suggestion would be that docs/README.md contains only:

Pointer to the (rendered) KSQL docs under docs.confluent.io = what we currently have at the beginning. This is for users of KSQL.

An explanation how to contribute to the KSQL documentation ("hey, we use Sphinx" etc.). This is for contributors of KSQL.

To keep things simple, I'd suggest to keep (1) in this README, but move most of (2) into the new top-level contributing.md file, and only link from docs/README.md to contributing.md. Otherwise docs/README.md is very busy and comes across as if you'd have to build the docs yourself if you fail to see the "Click here to go to docs.confluent.io" section at the top. That is, this README's content should not be 5% of what 99% of users need (where are the docs? in case I ended up in the docs/ folder), and 95% of what 1% of users need (how can I contribute to docs?).

yeah, makes sense - i'll make the update.

miguno · 2017-11-29T09:09:55Z

README.md

@@ -70,7 +70,7 @@ Whether you need help, want to contribute, or are just looking for the latest ne
 * Join the [Confluent Google group](https://groups.google.com/forum/#!forum/confluent-platform).

 # Contributing
-Contributions to the code, examples, documentation, etc, are very much appreciated. For more information, see the [contribution guidelines](/docs/contributing.md).
+Contributions to the code, examples, documentation, etc, are very much appreciated. For more information, see the [contribution guidelines](contributing.md).


Nit: Typically, the Markdown files are uppercased, i.e. README.md and CONTRIBUTING.md rather than readme.md and contributing.md.

i'm not aware of this convention. the filename is lowercase and the link should match the filename?

I mean, typically the .md filenames are uppercased. For example, our GH README is called README.md, not Readme.md or readme.md. Same for CONTRIBUTING.md vs. contributing.md. Again, a nit.

Feel free to ignore, Joel.

miguno · 2017-11-29T09:47:07Z

docs/concepts.rst

+The KSQL server runs the engine that executes KSQL queries, which
+includes the data processing as well as reading data from and writing
+data to the target Kafka cluster.
+


Based on user feedback, and while we're at it, I'd suggest to add a new paragraph here (as discussed with @bluemonk3y):

As described in the deployment documentation, servers can run in containers, VMs, or on bare-metal machines. You can add and remove multiple servers in the same resource pool to elastically scale in and out the processing of queries, and use different resource pools to achieve isolation of workloads.

There's some overlap here with what we want to cover in the section on deployment modes, but I think we should also mention the info above already here where most people are likely to read it.

miguno · 2017-11-29T09:50:28Z

docs/concepts.rst

+Standalone mode
+---------------
+
+In stand-alone mode, both the KSQL client and server components are


Nit consistency: standalone vs. stand-alone

miguno · 2017-11-29T10:30:01Z

docs/concepts.rst

+Client-server mode
+------------------
+
+In client-server mode, you can run a pool of KSQL servers on remote


Suggested:

In client-server mode, the KSQL servers are run separately from the KSQL CLI (the client). You can deploy servers on remote machines, VMs, or containers. The CLI then connects to these remote servers.

You can add and remove servers to/from the same resource pool during live operations to elastically scale in and out the processing of queries. You can use different resource pools to achieve isolation of workloads, for example by deploying separate pools for production and for testing and exploration.

miguno · 2017-11-29T10:32:14Z

docs/concepts.rst

+
+       $ ./bin/ksql-cli remote http://my-ksql-server:8090
+
+All KSQL servers (and their engines) share the work of processing KSQL


@bluemonk3y : We should update this section on how to actually pool the servers.

A KSQL server pool is one where all servers share the same 'command' topic. The default command topic is named: 'ksql__commands'. The command topic name can be changed by applying the following system properties to the ksqlserver.properties file.
ksql.service.id = ksql_
ksql.command.topic.suffix=commands

Good draft! Here's my slightly modified one:

KSQL servers that share the same "command" topic belong to the same resource pool. By default, a KSQL server uses the command topic named ksql__commands. To assign a server to a different pool than the default, change the ksql.command.topic.suffix setting in its configuration:

# Default value: `commands`. # # Changing this to `production_commands` as shown below will result in # the command topic named `ksql__production_commands` being used. ksql.command.topic.suffix = production_commands

miguno

Updated comment on "pools".

@miguno

WIP WIP WIP WIP WIP WIP WIP Merge change from PR 440 Review comments from @miguno and @hjafarpour comments addressed Change CLI version Change version 4.0 WIP WIP WIP WIP WIP WIP WIP WIP TOC

miguno · 2017-12-01T10:05:50Z

Why was this merged into 4.0.x? That was a mistake. It means our docs on GH are currently broken.

miguno · 2017-12-01T10:11:15Z

docs/quickstart/quickstart-docker.rst

+       $ cd ksql/docs/quickstart/
+       $ docker-compose up -d
+
+   After you have successfully started the Kafka cluster and started


This paragraph and the KSQL prompt doesn't belong here. You only see the KSQL ascii art prompt after you ran the docker-compose exec ksql-cli ksql-cli local --bootstrap-server kafka:29092 command in the next section on "Start KSQL".

Please also double-check that the non-docker Quickstart doesn't have the same mistake.

This reverts commit c488125.

joel-hamill · 2017-12-05T15:42:56Z

@miguno i confirmed with @hjafarpour that this should be merged to 4.0.x last week - and that is why it was merged. what exactly is broken?

* Revert "Make doc links point to GH repo (#496)" This reverts commit 0371a08. * Revert "DOCS-127 - Add KSQL doc to CP (#472)" This reverts commit c488125.

xvrl · 2018-05-29T14:49:40Z

looks like the PR broke links in our January blog post https://www.confluent.io/blog/ksql-january-release-streaming-sql-apache-kafka/

miguno · 2018-05-29T14:56:19Z

Good catch @xvrl -- I updated the blog post with new links.

miguno · 2018-05-29T14:58:46Z

PS: I also checked the Dec 2017, Feb 2018 blog posts - all good.

joel-hamill force-pushed the joel-hamill/merge-ksql-2-cp branch from 28758f9 to 313d98f Compare November 20, 2017 20:40

joel-hamill changed the title ~~[WIP] DOCS-127 - Add KSQL doc to CP~~ DOCS-127 - Add KSQL doc to CP Nov 20, 2017

joel-hamill requested review from a team and ewencp November 20, 2017 21:16

ewencp reviewed Nov 20, 2017

View reviewed changes

joel-hamill force-pushed the joel-hamill/merge-ksql-2-cp branch from e33f857 to f2a96c5 Compare November 20, 2017 23:30

joel-hamill requested a review from miguno November 20, 2017 23:30

ewencp approved these changes Nov 21, 2017

View reviewed changes

miguno suggested changes Nov 21, 2017

View reviewed changes

hjafarpour suggested changes Nov 21, 2017

View reviewed changes

bluemonk3y approved these changes Nov 27, 2017

View reviewed changes

hjafarpour reviewed Nov 28, 2017

View reviewed changes

hjafarpour approved these changes Nov 28, 2017

View reviewed changes

miguno suggested changes Nov 29, 2017

View reviewed changes

miguno reviewed Nov 29, 2017

View reviewed changes

joel-hamill force-pushed the joel-hamill/merge-ksql-2-cp branch from 3742e37 to 0b9074c Compare November 29, 2017 17:31

DOCS-127 - Add KSQL doc to CP

e58dcd5

WIP WIP WIP WIP WIP WIP WIP Merge change from PR 440 Review comments from @miguno and @hjafarpour comments addressed Change CLI version Change version 4.0 WIP WIP WIP WIP WIP WIP WIP WIP TOC

joel-hamill force-pushed the joel-hamill/merge-ksql-2-cp branch from 3a2f420 to e58dcd5 Compare November 29, 2017 17:38

Merge conflict

4db75a6

joel-hamill changed the base branch from 0.1.x to 4.0.x November 30, 2017 23:19

Merge conflicts

ce477a1

joel-hamill merged commit c488125 into 4.0.x Nov 30, 2017

joel-hamill deleted the joel-hamill/merge-ksql-2-cp branch November 30, 2017 23:21

miguno reviewed Dec 1, 2017

View reviewed changes

apurvam pushed a commit to apurvam/ksql that referenced this pull request Dec 4, 2017

Revert "DOCS-127 - Add KSQL doc to CP (confluentinc#472)"

01a7520

This reverts commit c488125.

apurvam pushed a commit to apurvam/ksql that referenced this pull request Dec 4, 2017

Revert "DOCS-127 - Add KSQL doc to CP (confluentinc#472)"

7697e4a

This reverts commit c488125.

apurvam added a commit that referenced this pull request Dec 5, 2017

Revert commits which broke the docs (#501)

5bf0d5c

* Revert "Make doc links point to GH repo (#496)" This reverts commit 0371a08. * Revert "DOCS-127 - Add KSQL doc to CP (#472)" This reverts commit c488125.

joel-hamill restored the joel-hamill/merge-ksql-2-cp branch December 5, 2017 23:31

joel-hamill mentioned this pull request Dec 5, 2017

DOCS-127 - Add KSQL docs to CP #504

Merged


		ARG DOCKER_REGISTRY

		FROM ${DOCKER_REGISTRY}confluentinc/docker-demo-base:3.3.0


		.. code:: bash

		cp ksql-clickstream-demo/demo/connect-config/null-filter-4.0.0-SNAPSHOT.jar <path-to-confluent-3.3.0>/share/java/kafka-connect-elasticsearch/

		Copyright 2017 Confluent Inc.

		CLI v0.1, Server v0.1 located at http://localhost:9098


		$ ./bin/ksql-cli remote http://my-ksql-server:8090

		All KSQL servers (and their engines) share the work of processing KSQL

DOCS-127 - Add KSQL doc to CP #472

DOCS-127 - Add KSQL doc to CP #472

Conversation

joel-hamill commented Nov 20, 2017 • edited Loading

ewencp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewencp left a comment

Choose a reason for hiding this comment

miguno left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bluemonk3y Nov 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bluemonk3y Nov 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bluemonk3y left a comment

Choose a reason for hiding this comment

joel-hamill commented Nov 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miguno left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miguno left a comment

Choose a reason for hiding this comment

miguno commented Dec 1, 2017

Choose a reason for hiding this comment

joel-hamill commented Dec 5, 2017

xvrl commented May 29, 2018

miguno commented May 29, 2018

miguno commented May 29, 2018

joel-hamill commented Nov 20, 2017 •

edited

Loading

bluemonk3y Nov 27, 2017 •

edited

Loading

bluemonk3y Nov 27, 2017 •

edited

Loading