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

WIP: feat(tour): add test descriptions to the 'Scaling up' section #654

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ffuerste
Copy link

Feature or Problem

Adding addtional and optional information to the Scaling up section of the quickstart.

Note: This PR is still a work in progress and I have created it for discussion purposes only. What is currently missing:

  • code snippets for the other languages used in the quickstart:
    • TinyGo
    • TypeScript
    • Python
  • explanation of why messages are no longer forwarded to the Hello component after multiple requests have been sent, although only a single replica was specified

Related Issues

closes #615

Consumer Impact

There will be no impact for the quickstart scope. Only additional (optional) information and tasks are added to make the Scaling up section more tangible.

Testing

Manual Verification

Checked the rendered docs in my browser.

@ffuerste ffuerste requested a review from a team as a code owner October 21, 2024 14:59
Copy link

netlify bot commented Oct 21, 2024

Deploy Preview for dreamy-golick-5f201e ready!

Name Link
🔨 Latest commit e386401
🔍 Latest deploy log https://app.netlify.com/sites/dreamy-golick-5f201e/deploys/67351032f27c66000809d5c0
😎 Deploy Preview https://deploy-preview-654--dreamy-golick-5f201e.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@brooksmtownsend brooksmtownsend left a comment

Choose a reason for hiding this comment

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

@ffuerste really love the additions here! I think the structure is looking good, just concerned about a couple of things

  1. We recently changed our quickstart a bit, so you might need a quick rebase to bring in the latest changes. Not much should've changed fundamentally on the scale&distribute page
  2. I notice the commands you use are seq and xargs, those are both present on Mac but are they present on Windows? (If not, that's fine, we'll just want to call it out)

Otherwise this is looking good, and it serves as good motivation + immediate payoff to scale up and handle more requests.

Comment on lines 113 to 115
```bash
nats sub "*.*.wrpc.>"
```
Copy link
Member

Choose a reason for hiding this comment

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

Something that might be good here too is using wash spy, since folks haven't needed the nats CLI yet

Copy link
Author

Choose a reason for hiding this comment

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

That's a very good point. Many thanks. Will look into it.

Copy link
Author

Choose a reason for hiding this comment

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

As suggested I replaced the nats CLI example with wash spy.

@@ -27,11 +139,32 @@ spec:
traits:
- type: spreadscaler
properties:
# Update the scale to 100
instances: 1 { // [!code --]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
instances: 1 { // [!code --]
replicas: 1 { // [!code --]

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the pointer. The snippet is taken from the current main branch of the rust http-hello-world. And in the corresponding commit it is stated, that replicas will soon be deprecated. So does it makes sense to change it to replicas?

However, in the docs for Application declarations also replicas is used. So do we keep the docs in sync or should we update the docs and switch to instances?

Copy link
Contributor

Choose a reason for hiding this comment

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

We've deprecated our use of replicas and should use instances in many places like in wasmCloud/wadm#314 and should definitely do so here in the docs as well.

Copy link
Author

Choose a reason for hiding this comment

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

Many thanks for the clarification.

Copy link
Member

@brooksmtownsend brooksmtownsend left a comment

Choose a reason for hiding this comment

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

Meant to request changes since this is a WIP PR

```

:::note[Why are not all requests forwarded to our component?]
TBD
Copy link
Author

Choose a reason for hiding this comment

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

I have tried to find out why only 4 requests are forwarded to the Hello component instead of all 10. Or why no more requests are generally forwarded to the component afterwards. Unfortunately, I have not yet found an answer. If I send further requests with curl, they are received by the http capability provider and forwarded to the component, but the messages are not received by the component because no log message is logged by it.

Output of wasmCloud logs for the curl request:

2024-10-26T21:35:08.420377Z DEBUG handle_request{target="rust_hello_world-http_component" scheme="http" authority="localhost:8080" request=Request { method: GET, uri: /?name=Alice, version: HTTP/1.1, headers: {"host": "localhost:8080", "user-agent": "curl/8.7.1", "accept": "*/*"}, body: Body(UnsyncBoxBody) }}: wrpc_transport::invoke: invoking function

The expected log messages about scaling the component are missing. Does anybody have a pointer how to debug this?

@ffuerste
Copy link
Author

ffuerste commented Nov 9, 2024

...
2. I notice the commands you use are seq and xargs, those are both present on Mac but are they present on Windows? (If not, that's fine, we'll just want to call it out)
...

I added an example also for Windows Powershell. Many thanks for the pointer.

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.

[DOCS] Proposal to extend the Quickstart - Scale and Distribute
3 participants