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

docs: Update multi-hop concept doc #4552

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Dan-Heath
Copy link
Contributor

@Dan-Heath Dan-Heath commented Mar 21, 2024

Copy link

@sdoyle88 sdoyle88 left a comment

Choose a reason for hiding this comment

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

Some commends and a few minor typo corrections

Boundary client connections egress the proxy chain at this worker to reach the target.

Note that ingress, intermediary, and egress are general ways to describe how the respective worker interfaces with resources.
A worker can act as more than one of those at a time.

## Multi-hop worker capabilities

Choose a reason for hiding this comment

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

Having read the preview, I'm thinking we move this section to be under the Introduction section, and we move the content from the Introduction section here and rename the header from Multi-hop worker capabilities to Multi-hop worker concepts. We talk about things upstream and downstream here currently, and if we move that to be under the introduction, then that is a good segway to better explain the concepts so users understand what was just mentioned in more detail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having read the preview, I'm thinking we move this section to be under the Introduction section, and we move the content from the Introduction section here and rename the header from Multi-hop worker capabilities to Multi-hop worker concepts. We talk about things upstream and downstream here currently, and if we move that to be under the introduction, then that is a good segway to better explain the concepts so users understand what was just mentioned in more detail.

Yep, great suggestion! I just moved these around and I think it flows nicely. I think this topic is much improved 🎉

I am wondering if we should expand on why people would want to configure multi-hop? We briefly address the benefits in the first para, but I'm wondering if it could be stronger.

Choose a reason for hiding this comment

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

I reread the intro and thought about it some more, and honestly, Im coming up without any improvements. The value of multi-hop workers seems clear to me with that intro, but I also have been working with multi-hop workers a lot so perhaps another set of eyes on it would be good?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! I would like to bring in some other reviewers. Thank you for all your help @sdoyle88 . Robin and I still need to review your updates to the worker procedure too. We'll do those things and get back to you.

Co-authored-by: Sean Doyle <sdoyle88@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants