Skip to content

docs: Add extension categories to extension docs#14721

Merged
htuch merged 115 commits intoenvoyproxy:mainfrom
phlax:proto-doc-extensions
Feb 17, 2021
Merged

docs: Add extension categories to extension docs#14721
htuch merged 115 commits intoenvoyproxy:mainfrom
phlax:proto-doc-extensions

Conversation

@phlax
Copy link
Copy Markdown
Member

@phlax phlax commented Jan 15, 2021

Signed-off-by: Ryan Northey ryan@synca.io

Commit Message: docs: Add extension categories to extension docs
Additional Description:

Add information about known extension categories (types) to extension documentation

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: Ryan Northey <ryan@synca.io>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
API shepherd assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #14721 was opened by phlax.

see: more, trace.

@phlax phlax changed the title docs: Add extension types to extension docs [WIP] docs: Add extension types to extension docs Jan 15, 2021
@phlax phlax marked this pull request as draft January 15, 2021 12:02
phlax added 5 commits January 15, 2021 12:27
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Copy link
Copy Markdown
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.

Nice, this looks like the right direction.

@htuch htuch assigned htuch and unassigned markdroth Jan 15, 2021
Signed-off-by: Ryan Northey <ryan@synca.io>
Base automatically changed from master to main January 15, 2021 23:02
phlax added 6 commits January 18, 2021 11:11
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
phlax added 5 commits January 22, 2021 13:29
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
for _k, _v in EXTENSION_DB.items():
for _cat in _v['categories']:
EXTENSION_CATEGORIES.setdefault(_cat, [])
EXTENSION_CATEGORIES[_cat].append(_k)
Copy link
Copy Markdown
Member Author

@phlax phlax Feb 16, 2021

Choose a reason for hiding this comment

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

@htuch i removed the script and shifted the code here

this simplifies/reduces the code and also parses the extension db only once (within this script), which should improve the efficiency of protodoc (if only by a tiny fraction im realising)

i have used setdefault as defaultdict adds some complexity and would prevent KeyErrors from being raised below

Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax changed the title docs: Add extension types to extension docs docs: Add extension categories to extension docs Feb 16, 2021
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Copy Markdown
Member Author

phlax commented Feb 16, 2021

@htuch nits addressed (hopefully)

@phlax phlax requested a review from htuch February 16, 2021 15:09
@phlax
Copy link
Copy Markdown
Member Author

phlax commented Feb 16, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #14721 (comment) was created by @phlax.

see: more, trace.

except KeyError as e:
raise ProtodocError(f"\n\nUnable to find extension category: {extension_category}\n\n")
anchor = FormatAnchor('extension_category_' + extension_category)
extensions = FormatExtensionList(sorted(extensions))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

move sorted into the extension list func

Copy link
Copy Markdown
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.

LGTM, thanks! So cool.

@htuch htuch merged commit 7adc039 into envoyproxy:main Feb 17, 2021
@mattklein123
Copy link
Copy Markdown
Member

@phlax this is absolutely amazing. Well done. For future work, I'm wondering if we should do the following:

  1. Also document the type_url?
  2. Potentially auto generate YAML snippets of the name/type_url? I think there might still be confusion on how to actually do the configuration.

We should track these improvements in other issues? WDYT?

@phlax
Copy link
Copy Markdown
Member Author

phlax commented Feb 18, 2021

We should track these improvements in other issues? WDYT?

i think it sounds like a good idea - altho from where i am working right now, im not entirely clear if we are talking about documenting end use (ie @type) or dev (ie type_url + value) - i guess the former

so, there are a couple of existing tickets in relation to this.

i opened this one:

#13531

mostly born from frustration of finding configuration examples using the older config and finding difficult to find information on how to upgrade, or where the definitions for the newer configs could be found

for the first part of the problem - upgrading - i guess its a lessening issue as time passes - perhaps we should just close it and focus forward

the second part of it is where to get accurate info on extensions etc, which i think is the existing ticket that you had opened previously:

#13167

i think that one has a partial fix with this PR - lets use it to track further improvements on the extension <> category docs

htuch pushed a commit that referenced this pull request Feb 26, 2021
Some of the categories etc added in #14721 were not exactly correct and/or consistent

Also there is now some duplication from where links/info have been added manually previously

This PR should address these issues

Ref #13167

Signed-off-by: Ryan Northey <ryan@synca.io>
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.

4 participants