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 "Traffic" Plugin docs 3 #7064

Merged
merged 7 commits into from
May 20, 2022

Conversation

pottekkat
Copy link
Contributor

Signed-off-by: Navendu Pottekkat navendupottekkat@gmail.com

Description

Updates the documentation of the "traffic-split", "request-id", "proxy-control", and "client-control" Plugins.

Child PR of #6734

@pottekkat
Copy link
Contributor Author

// @hf400159 @kwanhur @avinal @yzeng25 @juzhiyuan Please review when you are available.

Signed-off-by: Navendu Pottekkat <navendupottekkat@gmail.com>
docs/en/latest/plugins/client-control.md Outdated Show resolved Hide resolved
docs/en/latest/plugins/client-control.md Outdated Show resolved Hide resolved
docs/en/latest/plugins/proxy-control.md Outdated Show resolved Hide resolved
data_machine_interval: 10
```

:::warning
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see "warning" listed on https://docusaurus.io/docs/markdown-features/admonitions, please specify which one you'd like to use. Personally I think :::caution should do the work.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you decide to stick with "waring", you can also do a trick like :::caution Warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it was supposed to be :::caution WARNING. I missed it. Thank you for pointing it out.

docs/en/latest/plugins/request-id.md Outdated Show resolved Hide resolved
Copy link
Contributor

@guitu168 guitu168 left a comment

Choose a reason for hiding this comment

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

other LGTM 👍

@@ -23,19 +29,23 @@ title: proxy-control

## Description

The `proxy-control` plugin dynamically controls the behavior of Nginx to proxy.
The proxy-control Plugin dynamically controls the behavior of the Nginx proxy.
Copy link
Contributor

@guitu168 guitu168 May 17, 2022

Choose a reason for hiding this comment

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

Suggested change
The proxy-control Plugin dynamically controls the behavior of the Nginx proxy.
The proxy-control Plugin dynamically controls the behavior of the NGINX proxy.

Copy link
Contributor

Choose a reason for hiding this comment

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

I found on the NGINX official website, it should be NGINX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we started using Nginx and kept using it. We might need to change this everywhere if this is desired.

Signed-off-by: Navendu Pottekkat <navendupottekkat@gmail.com>
Signed-off-by: Navendu Pottekkat <navendupottekkat@gmail.com>
@pottekkat
Copy link
Contributor Author

@yzeng25 Made the changes as suggested.

yzeng25
yzeng25 previously approved these changes May 19, 2022
Copy link
Contributor

@yzeng25 yzeng25 left a comment

Choose a reason for hiding this comment

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

LGTM

guitu168
guitu168 previously approved these changes May 19, 2022
Copy link
Contributor

@guitu168 guitu168 left a comment

Choose a reason for hiding this comment

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

LGTM


The examples below shows different use cases for using the `traffic-split` Plugin.

### Grayscale/Canary release
Copy link
Member

Choose a reason for hiding this comment

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

Please use Canary Release instead

Copy link
Member

Choose a reason for hiding this comment

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

Others LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So no need for mentioning Grayscale or Canary?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, only using the Canary release is enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, done!

Signed-off-by: Navendu Pottekkat <navendupottekkat@gmail.com>
@pottekkat pottekkat dismissed stale reviews from guitu168 and yzeng25 via 6321221 May 20, 2022 03:46
Copy link
Member

@SylviaBABY SylviaBABY left a comment

Choose a reason for hiding this comment

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

LGTM

@juzhiyuan juzhiyuan merged commit e3a9eb6 into apache:master May 20, 2022
Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request May 20, 2022
Signed-off-by: Navendu Pottekkat <navendupottekkat@gmail.com>
@pottekkat pottekkat deleted the docs/plugin-traffic-3/6734 branch May 23, 2022 05:46
hongbinhsu pushed a commit to fitphp/apix that referenced this pull request May 24, 2022
* upsgrteam/master: (351 commits)
  fix(proxy-cache): bypass when method mismatch cache_method (apache#7111)
  chore(script): support to install dependencies under arm64 (apache#7091)
  chore(ci): use the latest build script for apisix-base (apache#7090)
  fix(batch-requests): ignore "unix:" in the configuration (apache#7106)
  fix: install dependencies issues (apache#7092)
  feat(ops): use lua libs to backup config file insteadof shell command (apache#7048)
  test: reduce CI failure caused by flaky tests (apache#7085)
  chore(ci): move set_dns.sh to ci dir (apache#7089)
  feat: release 2.14.0 (apache#7057)
  docs: update "Tracers" Plugins (apache#7086)
  docs: update "Traffic" Plugin docs 3 (apache#7064)
  docs: update "Serverless" Plugins (apache#7076)
  feat(ops): check process running with posix.signal insteadof lsof (apache#7006)
  docs: modify how-to-build filename (apache#7087)
  docs: fix link of hot-reload in docs (apache#7081)
  chore(ci): apt update before install (apache#7080)
  docs: add pubsub develop example for kafka (apache#7059)
  ci: enable rebase in some situation (apache#7074)
  fix: redirect http to https but port not change (apache#7065)
  ci: make it pass under OpenResty 1.21 (apache#7067)
  ...
spacewander pushed a commit that referenced this pull request Jun 30, 2022
Signed-off-by: Navendu Pottekkat <navendupottekkat@gmail.com>
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.

5 participants