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: move xds protocol to rst #6670

Merged
merged 6 commits into from
Apr 22, 2019

Conversation

ramaraochavali
Copy link
Contributor

Description: This PR moves the xds protocol from md to rst.
Risk Level: Low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A
Fixes #6338

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
@@ -13,3 +13,4 @@ Introduction
getting_help
version_history
deprecated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added this new line to be consistent with all other files

@ramaraochavali
Copy link
Contributor Author

@mattklein123 @htuch PTAL

@ramaraochavali
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #6670 (comment) was created by @ramaraochavali.

see: more, trace.

@htuch htuch self-assigned this Apr 22, 2019
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Thanks @ramaraochavali this is a nice improvement to the API docs situation. A few comments.
/wait

@@ -19,4 +19,5 @@ Envoy documentation
operations/operations
extending/extending
api-v2/api
api-docs/xds_protocol
Copy link
Member

Choose a reason for hiding this comment

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

I think we should have a single top-level "API", and nested under this the xDS wire and API reference entries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

services and their corresponding APIs are referred to as *xDS*.
Resources are requested via *subscriptions*, by specifying a filesystem
path to watch, initiating gRPC streams or polling a REST-JSON URL. The
latter two methods involve sending requests with a :ref:`Discovery Request <envoy_api_msg_DiscoveryRequest>`
Copy link
Member

Choose a reason for hiding this comment

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

Why did you add a space here between "Discovery" and "Request" (and "Response" below)? Ideally we keep the content exactly the same as the previous, no changes to things like naming etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was by mistake. I removed the spaces now


In this sequence diagram, and below, the following format is used to abbreviate messages:

- *DiscoveryRequest*: (V=`version_info`,R=`resource_names`,N=`response_nonce`,T=`type_url`)
Copy link
Member

Choose a reason for hiding this comment

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

The backticks are visible in the rendered docs here.

Each stream has its own notion of versioning, there is no shared
versioning across resource types. When ADS is not used, even each
resource of a given resource type may have a distinct version, since the
Envoy API allows distinct EDS/RDS resources to point at different :ref:`Config Sources <envoy_api_msg_core.ConfigSource>`.
Copy link
Member

Choose a reason for hiding this comment

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

This is another place a space has been inserted inside an identifier.

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
@ramaraochavali
Copy link
Contributor Author

@htuch changed. PTAL.

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Thanks, this is awesome.

@htuch htuch merged commit a3fe3c6 into envoyproxy:master Apr 22, 2019
@ramaraochavali ramaraochavali deleted the fix/xds_protocol branch April 23, 2019 02:03
mpuncel added a commit to mpuncel/envoy that referenced this pull request Apr 24, 2019
* master:
  docs: add extension policy (envoyproxy#6678)
  ext_authz: added ability to detect partial request body data (envoyproxy#6583)
  version_history.rst: jwt_authn change missed 1.10.0 (envoyproxy#6684)
  docs: fix link in pull request template (envoyproxy#6679)
  Explicitly convert absl::string_view to std::string. (envoyproxy#6687)
  docs: improving watermark docs/comments (envoyproxy#6683)
  http filter: add CSRF filter (envoyproxy#6470)
  event: reintroduce dispatcher stats (envoyproxy#6659)
  security: postmortem for CVE-2019-990[01] (envoyproxy#6597)
  Improve build rules for (test only) library quic_port_utils. (envoyproxy#6672)
  spell check: skip unsupported extensions when called with a file (envoyproxy#6648)
  Changed TestHooks to ListenerHooks (envoyproxy#6642)
  proto: move extension-specific linking validation into extensions (envoyproxy#6657)
  stats: add/test heterogenous set of StatNameStorage objects. (envoyproxy#6504)
  docs: move xds protocol to rst (envoyproxy#6670)
  fix version history order (envoyproxy#6671)

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
@phylake
Copy link

phylake commented Apr 30, 2019

All the links are broken now, and the whole thing is harder to read. I'm guessing those things are related

image

@moderation
Copy link
Contributor

@phylake the finished product is rendered at https://www.envoyproxy.io/docs/envoy/latest/api-docs/xds_protocol and the links look OK and work as expected.

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.

Move XDS_PROTOCOL.md into sphinx docs
4 participants