Create Hello World program with Kotlin #381

pzmrzy · 2018-09-07T17:34:29Z

Proposed Changes

Add a hello world example in Kotlin

pzmrzy · 2018-09-07T20:47:07Z

/assign @mattmoor

mattmoor · 2018-09-21T16:35:11Z

/ok-to-test

grantr

Thanks @pzmrzy!

/lgtm

Sample code looks good to me, and it worked when I ran through it.

/assign @samodell
for approval.

grantr · 2018-09-21T17:37:33Z

serving/samples/helloworld-kotlin/README.md

+    }
+    ```
+
+3. Create a new file, `build.gradle` and copy the following setting


Can you mention the switch back to the root directory for this and later files?

grantr · 2018-09-21T17:37:50Z

serving/samples/helloworld-kotlin/README.md

+
+4. To find the IP address for your service, use
+   `kubectl get svc knative-ingressgateway -n istio-system` to get the ingress IP for your
+   cluster. If your cluster is new, it may take sometime for the service to get asssigned


typo: assigned

grantr · 2018-09-21T17:45:25Z

serving/samples/helloworld-kotlin/README.md

+    ```shell
+    kubectl get svc knative-ingressgateway -n istio-system
+
+    NAME                     TYPE           CLUSTER-IP     EXTERNAL-IP      PORT(S)                                      AGE


The output of the command should be in a separate shell block so users can copy the entire command block without interpreting its contents.

grantr · 2018-09-21T17:45:58Z

serving/samples/helloworld-kotlin/README.md

+5. To find the URL for your service, use
+    ```
+    kubectl get ksvc helloworld-kotlin  -o=custom-columns=NAME:.metadata.name,DOMAIN:.status.domain
+    NAME                DOMAIN


Separate shell block

grantr · 2018-09-21T17:46:04Z

serving/samples/helloworld-kotlin/README.md

+
+    ```shell
+    curl -H "Host: helloworld-kotlin.default.example.com" http://{IP_ADDRESS}
+    Hello World: Kotlin Sample v1


Separate shell block

grantr · 2018-09-21T17:48:35Z

/unassign @grantr

…example

grantr · 2018-09-27T20:30:46Z

/lgtm

vdemeester · 2018-09-28T08:42:28Z

serving/samples/helloworld-kotlin/Dockerfile

+
+CMD ["gradle"]
+
+ENV GRADLE_HOME /opt/gradle


@pzmrzy is there a reason for not using gradle official image(s) ? (as it really does the same thing https://github.com/keeganwitt/docker-gradle/blob/bd6b0605196488734d8d61d6a00a4ca96846d2e7/jdk8/Dockerfile)

@vdemeester I think no, I did't find this before, I just search on github for a gradle docker image

@pzmrzy I guess we can do a follow-up to use an official image then 😉

@vdemeester Just update the image and test again

@pzmrzy Can we use the official gradle image for this? Lines 1-34 can be replaced by:

FROM gradle

…example

imikushin · 2018-10-19T22:27:04Z

@pzmrzy Let's see what we can do to get this in ;)

Can you resolve the merge conflicts first?

/assign

imikushin

/lgtm

imikushin · 2018-10-20T00:29:57Z

@samodell Can you approve?

imikushin · 2018-10-23T22:31:01Z

@RichieEscarez Can you take another look at this? It's been a while

samodell · 2018-10-25T17:46:36Z

Hey @imikushin -- we recently merged in some updates to the Hello World samples; they've been rewritten so they aren't hard coded to listen on port 8080. See this issue for more information: #456 It looks to me like your sample is hard-coded to listen on port 8080? (I'm definitely not a Kotlin expert, so let me know if that's not the case.) The "Hello World" sections were also slightly revamped; see the changes on lines 4-5 in the README of this PR for an example: https://github.com/knative/docs/pull/462/files#diff-99942f6b9d4f507ea57838988d5f8cbc
I'll also do a quick review of the writing here for readability later today. Once all that's done, we'll get this merged in! Thanks for your patience.

imikushin · 2018-10-25T19:04:47Z

serving/samples/helloworld-kotlin/service.yaml

+      revisionTemplate:
+        spec:
+          container:
+            image: docker.io/{username}/helloworld-kotlin


docker.io/ should probably be omitted:
I saw image pull errors with docker.io/imikushin/helloworld-kotlin, but imikushin/helloworld-kotlin worked just fine 🤷‍♂️

It works fine for me with docker.io, other samples also have this.

pzmrzy · 2018-10-25T21:33:51Z

@samodell Just updated!

samodell

A few more changes.

samodell · 2018-10-25T22:16:15Z

serving/samples/helloworld-kotlin/README.md

@@ -0,0 +1,213 @@
+# Hello World - Kotlin sample
+
+A simple web app written in Kotlin using Ktor that you can use for testing.


Suggestion: add a link to more info about the Ktor framework

samodell · 2018-10-25T22:28:16Z

serving/samples/helloworld-kotlin/README.md

+   an external IP address.
+
+    ```shell
+    kubectl get svc knative-ingressgateway -n istio-system


We ended up reverting the changes that exposed the short names (svc, for example) because they're not in the 0.1 release. To fix, replace "svc" and "ksvc" with "services.serving.knative.dev" and delete the Note about the short names, further down.

Thanks for the comment, just updated.

samodell · 2018-10-25T23:14:28Z

serving/samples/helloworld-kotlin/README.md

+    curl -H "Host: helloworld-kotlin.default.example.com" http://{IP_ADDRESS}
+    ```
+    ```shell
+    Hello World: Kotlin Sample v1


The other changes made above mean this output should now be "Hello Kotlin Sample v1".

knative-prow-robot · 2018-10-26T18:33:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: imikushin, pzmrzy, samodell, vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [samodell]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

samodell · 2018-10-26T18:33:11Z

/lgtm

Create Hello World program with Kotlin

47e068d

knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 7, 2018

knative-prow-robot requested review from RichieEscarez and samodell September 7, 2018 17:34

knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 7, 2018

knative-prow-robot assigned mattmoor Sep 7, 2018

RichieEscarez assigned mdemirhan Sep 10, 2018

RichieEscarez added triage/needs-eng-input Engineering input is requested needs edits labels Sep 10, 2018

knative-prow-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 21, 2018

mattmoor assigned grantr and unassigned mattmoor and mdemirhan Sep 21, 2018

grantr reviewed Sep 21, 2018

View reviewed changes

knative-prow-robot assigned samodell Sep 21, 2018

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 21, 2018

knative-prow-robot unassigned grantr Sep 21, 2018

pzmrzy added 2 commits September 21, 2018 11:06

Merge branch 'master' of https://github.com/knative/docs into kotlin_…

a82bee3

…example

Modify Readme by addressing Grant's comments

f597989

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 21, 2018

knative-prow-robot assigned grantr Sep 27, 2018

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2018

vdemeester reviewed Sep 28, 2018

View reviewed changes

vdemeester approved these changes Oct 1, 2018

View reviewed changes

pzmrzy added 2 commits October 1, 2018 10:45

Merge branch 'master' of https://github.com/knative/docs into kotlin_…

e7d9812

…example

Update to gradle official image

965b0f1

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2018

knative-prow-robot assigned imikushin Oct 19, 2018

pzmrzy and others added 2 commits October 19, 2018 15:29

Merge branch 'master' into kotlin_example

9a7adfd

Update dockerfile by addressing ivan's comment

31c8294

imikushin approved these changes Oct 20, 2018

View reviewed changes

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2018

imikushin reviewed Oct 25, 2018

View reviewed changes

Read port from env by addressing samodell's comment

5961cf6

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 25, 2018

samodell suggested changes Oct 25, 2018

View reviewed changes

pzmrzy added 2 commits October 25, 2018 16:28

Update Readme by addressing samodell's comment

7435aaa

svc->service

bc04518

samodell approved these changes Oct 26, 2018

View reviewed changes

knative-prow-robot added the approved label Oct 26, 2018

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 26, 2018

knative-prow-robot merged commit c9430ee into knative:master Oct 26, 2018

		@@ -0,0 +1,213 @@
		# Hello World - Kotlin sample

		A simple web app written in Kotlin using Ktor that you can use for testing.

Create Hello World program with Kotlin #381

Create Hello World program with Kotlin #381

Conversation

pzmrzy commented Sep 7, 2018

Proposed Changes

Uh oh!

pzmrzy commented Sep 7, 2018

Uh oh!

mattmoor commented Sep 21, 2018

Uh oh!

grantr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grantr commented Sep 21, 2018

Uh oh!

grantr commented Sep 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imikushin commented Oct 19, 2018

Uh oh!

imikushin left a comment

Choose a reason for hiding this comment

Uh oh!

imikushin commented Oct 20, 2018

Uh oh!

imikushin commented Oct 23, 2018

Uh oh!

samodell commented Oct 25, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pzmrzy commented Oct 25, 2018

Uh oh!

samodell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knative-prow-robot commented Oct 26, 2018

Uh oh!

samodell commented Oct 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants