Paulyuk/hello world refactor

[paulyuk]

Description

This addresses the proposal to refactor ./hello-world quickstart into node and python subfolders. This is closer to what other quickstarts do, to what real life code looks like, and it enables downstream Devops benefits like having seperate Dockerfiles or builds.

The following changes were made:

• refactored all Node.js code and assets for nodeapp into ./hello-world/node
• refactored all Python code and assets for pythonapp into ./hello-world/python
• updated README to match new paths

Issue reference

Fixes #477

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

• The quickstart code compiles correctly
• You've tested new builds of the quickstart if you changed quickstart code
• You've updated the quickstart's README if necessary

[msfussell]

1. Dapr does not requires docker to host the applications and this implies it requires this. It is important the developer understand that Dapr runs executables, not code in containers. This quickstart should not have docker file added, since this is about using Dapr to launch exes.
2. The Hello-World K8s example (which has to have containers) is where the docker files for these apps are including these instructions https://github.com/dapr/quickstarts/tree/master/hello-kubernetes#deploying-your-code
3. Even if we were to add dockerfiles, these files are different from the Hello-World K8s example, and should be the same.
4. Adding Tye to this quickstart should be separated out. This is best placed as a documentation example in the IDE section of the docs and is the best place for people to learn about Tye, not from a quickstart. For example I would not include Bridge for K8s into the Hello-World K8s example and the VS Code extension in not include currently, which is also in preview. (and specifically the Tye build make file instructions target Linux and instructions on using Windows machines should be included, presumably saying this requires WSL2)

[paulyuk]

1. Dapr does not requires docker to host the applications and this implies it requires this. It is important the developer understand that Dapr runs executables, not code in containers. This quickstart should not have docker file added, since this is about using Dapr to launch exes.
2. The Hello-World K8s example (which has to have containers) is where the docker files for these apps are including these instructions https://github.com/dapr/quickstarts/tree/master/hello-kubernetes#deploying-your-code
3. Even if we were to add dockerfiles, these files are different from the Hello-World K8s example, and should be the same.
4. Adding Tye to this quickstart should be separated out. This is best placed as a documentation example in the IDE section of the docs and is the best place for people to learn about Tye, not from a quickstart. For example I would not include Bridge for K8s into the Hello-World K8s example and the VS Code extension in not include currently, which is also in preview. (and specifically the Tye build make file instructions target Linux and instructions on using Windows machines should be included, presumably saying this requires WSL2)

Hey @msfussell, thank you for the feedback. Ok we can keep this clean and focused on inner loop, and break build and deploy into separate steps. Tye is an open source tool that is really handy for all languages we use in Dapr, but let's also keep that separate. I haven't written new docs for these, so we'll leave that to a future PR.

Now this PR is about:

• refactored nodeapp and pythonapp code and dependencies into their own folders
• updated readme with paths.

That's it. Better?

[willtsai]

I noticed that you've moved both hello-world/sample.http and hello-world/sample.json into the hello-world/python directory. While that makes sense for the structure of the source code since the python app initiates the orders, I actually think these sample "orders" should be kept in the hello-world/node directory instead. Since the user of this quickstart will use the sample.* files to test their node deployment, we should house these files in the node directory for easier access immediately following their node dapr app deployment. Thoughts?

[paulyuk]

I noticed that you've moved both hello-world/sample.http and hello-world/sample.json into the hello-world/python directory. While that makes sense for the structure of the source code since the python app initiates the orders, I actually think these sample "orders" should be kept in the hello-world/node directory instead. Since the user of this quickstart will use the sample.* files to test their node deployment, we should house these files in the node directory for easier access immediately following their node dapr app deployment. Thoughts?

Hi Will! thank you for the review and feedback. Good catch. It's interesting that sample data is a hard requirement for pythonapp, but it's also a soft dependency of nodeapp since we guide folks to call the invoke CLI command with it. Let's call on some creativity then..

How about this:

sample.http MOVES to \node folder because it's the documented way to test the nodeapp service from any REST client, curl etc. Note that a sample data is self-contained in this file so you don't need second sample.json.

sample.json is COPIED (yes there will be two copies) since is really a hard dependency of pythonapp client, but also a soft dependency of the doc for nodeapp. I'm a bit of a stickler about data being co-located with its microservice, and not leaking out. The only other alternative I can see is moving sample.json and sample.http up to the root of hello-world, and change docs to say ../samples.json as path. That seems less ideal to me.

[willtsai]

I noticed that you've moved both hello-world/sample.http and hello-world/sample.json into the hello-world/python directory. While that makes sense for the structure of the source code since the python app initiates the orders, I actually think these sample "orders" should be kept in the hello-world/node directory instead. Since the user of this quickstart will use the sample.* files to test their node deployment, we should house these files in the node directory for easier access immediately following their node dapr app deployment. Thoughts?

Hi Will! thank you for the review and feedback. Good catch. It's interesting that sample data is a hard requirement for pythonapp, but it's also a soft dependency of nodeapp since we guide folks to call the invoke CLI command with it. Let's call on some creativity then..

How about this:

sample.http MOVES to \node folder because it's the documented way to test the nodeapp service from any REST client, curl etc. Note that a sample data is self-contained in this file so you don't need second sample.json.

sample.json is COPIED (yes there will be two copies) since is really a hard dependency of pythonapp client, but also a soft dependency of the doc for nodeapp. I'm a bit of a stickler about data being co-located with its microservice, and not leaking out. The only other alternative I can see is moving sample.json and sample.http up to the root of hello-world, and change docs to say ../samples.json as path. That seems less ideal to me.

@paulyuk - I agree with this approach. While having two copies of sample.json is redundant, I think it's necessary for the sake of convenience in the quickstart steps (i.e. no need to cd back and forth between /node and /python). Plus, I don't expect the data model to be changed much for these simple apps, so I think this is acceptable.

[willtsai]

LTGM

[msfussell]

Can you make this sentence and the node one "Open a terminal window and navigate" consistent. One says Command Line terminal and the other terminal window.

[paulyuk]

"command line terminal" was a preexisting issue, but I was happy to fix while in there. That's all done now. We're going with "new terminal" everywhere.

[msfussell]

Did you intend to go to the hello-world/K8s quickstart and "improve" the container Dockerfile with your suggestions?
Also these folder changes are simply to make it consistent with the K8s/Hello World example I presume?

[paulyuk]

Did you intend to go to the hello-world/K8s quickstart and "improve" the container Dockerfile with your suggestions? Also these folder changes are simply to make it consistent with the K8s/Hello World example I presume?

@msfussell Yes I plan to do that k8s dockerfile work separately in another PR focused on deployment options including dockerized ones. These folder changes simply add consistency, but also mirror what a node or python developer is used to, as well as what you'd want to see out of tiny autonomous microservices.

[msfussell]

Changes LGTM