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: Remove api v2 #16077

Merged
merged 11 commits into from
May 2, 2021
Merged

docs: Remove api v2 #16077

merged 11 commits into from
May 2, 2021

Conversation

phlax
Copy link
Member

@phlax phlax commented Apr 20, 2021

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

Commit Message: docs: Remove api v2
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

@phlax phlax marked this pull request as draft April 20, 2021 09:29
@phlax
Copy link
Member Author

phlax commented Apr 20, 2021

@htuch as your comment in docs/build.sh suggests the biggest challenge to removing the v2 build is with the version history - i think that will require its own solution

there are a few more failures:

  • api/xds_protocol.rst throws a ton of errors
  • configuration/arch_overview/operations and maybe a few others - small number of errors

if we can agree a way forward for the version history (perhaps linking to old docs ?) then im happy to go through and update

not sure about the xds protocol doc - but i guess its a case of updating/rewriting it to point only to v3 xds

the other errors are pretty small in number, and im guessing we again just need to update the relevant docs

@htuch
Copy link
Member

htuch commented Apr 20, 2021

Yeah, for version history, let's replace these with HTTP references to the 1.17.0 v2 API docs. It should be possible to sed this, since the references follow a regular pattern, e.g. envoy_api vs. envoy_v3_api.

The rest of the docs should just point to the v3 counterparts.

@htuch htuch self-assigned this Apr 20, 2021
@phlax
Copy link
Member Author

phlax commented Apr 21, 2021

@htuch
Copy link
Member

htuch commented Apr 21, 2021

@phlax output looks good, happy to review when in ready state.

@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #16077 was synchronize by phlax.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Apr 24, 2021
@phlax phlax force-pushed the docs-remove-api-v2 branch 7 times, most recently from 37c8ebf to 679c18f Compare April 27, 2021 10:08
@phlax
Copy link
Member Author

phlax commented Apr 27, 2021

@htuch once #16155 has landed there will be one outstanding issue with this - the not_so_simple_http_cache issue - once that is resolved we should be able to remove v2 from the docs

@phlax phlax force-pushed the docs-remove-api-v2 branch 2 times, most recently from 827e328 to 483fbd3 Compare April 27, 2021 16:37
@repokitteh-read-only
Copy link

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

🐱

Caused by: #16077 was synchronize by phlax.

see: more, trace.

@phlax phlax force-pushed the docs-remove-api-v2 branch 3 times, most recently from 264bfb3 to a3c4a76 Compare May 1, 2021 11:55
phlax added 4 commits May 2, 2021 16:39
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 6 commits May 2, 2021 16:39
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>
@@ -102,6 +102,7 @@ stages:
BAZEL_REMOTE_INSTANCE: projects/envoy-ci/instances/default_instance
GCP_SERVICE_ACCOUNT_KEY: $(GcpServiceAccountKey)
displayName: "Generate docs"
continueOnError: true
Copy link
Member Author

Choose a reason for hiding this comment

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

remove

Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax changed the title [WIP] docs: Remove api v2 docs: Remove api v2 May 2, 2021
@phlax phlax marked this pull request as ready for review May 2, 2021 17:27
@phlax phlax requested a review from htuch May 2, 2021 17:27
@phlax
Copy link
Member Author

phlax commented May 2, 2021

@htuch this doesnt remove all of the v2 references - but it gets most of them and prevents the v2 proto -> rst build - so its a pretty good start i think

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.

LGTM, thanks!

@htuch htuch merged commit e22e4a1 into envoyproxy:main May 2, 2021
@alyssawilk
Copy link
Contributor

hmmm, I noticed over on #15926 if I fix_proto and fix_format it adds these back:

--- a/api/BUILD
+++ b/api/BUILD
@@ -133,12 +133,18 @@ proto_library(
"//envoy/config/common/matcher/v3:pkg",
"//envoy/config/core/v3:pkg",
"//envoy/config/endpoint/v3:pkg",

  •    "//envoy/config/filter/thrift/router/v2alpha1:pkg",
       "//envoy/config/grpc_credential/v3:pkg",
    
  •    "//envoy/config/health_checker/redis/v2:pkg",
       "//envoy/config/listener/v3:pkg",
       "//envoy/config/metrics/v3:pkg",
       "//envoy/config/overload/v3:pkg",
       "//envoy/config/ratelimit/v3:pkg",
       "//envoy/config/rbac/v3:pkg",
    
  •    "//envoy/config/resource_monitor/fixed_heap/v2alpha:pkg",
    
  •    "//envoy/config/resource_monitor/injected_resource/v2alpha:pkg",
    
  •    "//envoy/config/retry/omit_canary_hosts/v2:pkg",
    
  •    "//envoy/config/retry/previous_hosts/v2:pkg",
       "//envoy/config/route/v3:pkg",
       "//envoy/config/tap/v3:pkg",
       "//envoy/config/trace/v3:pkg",
    

I'll just stash those changes and see if CI is happy, but I think we need to fix some auto-gen thing to avoid these being added back

@phlax
Copy link
Member Author

phlax commented May 5, 2021

@alyssawilk i think the fix is #16330

alyssawilk pushed a commit that referenced this pull request May 5, 2021
Commit Message: api: Fix generated_api_shadow BUILD
Additional Description:

Some changes were made to the api/BUILD file in #16077 which were not propagated to generated_api_shadow
this is also a possible reason that a dev reported an issue with ci formatting

Signed-off-by: Ryan Northey <ryan@synca.io>
gokulnair pushed a commit to gokulnair/envoy that referenced this pull request May 6, 2021
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Gokul Nair <gnair@twitter.com>
gokulnair pushed a commit to gokulnair/envoy that referenced this pull request May 6, 2021
Commit Message: api: Fix generated_api_shadow BUILD
Additional Description:

Some changes were made to the api/BUILD file in envoyproxy#16077 which were not propagated to generated_api_shadow
this is also a possible reason that a dev reported an issue with ci formatting

Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Gokul Nair <gnair@twitter.com>
gokulnair pushed a commit to gokulnair/envoy that referenced this pull request May 6, 2021
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Gokul Nair <gnair@twitter.com>
gokulnair pushed a commit to gokulnair/envoy that referenced this pull request May 6, 2021
Commit Message: api: Fix generated_api_shadow BUILD
Additional Description:

Some changes were made to the api/BUILD file in envoyproxy#16077 which were not propagated to generated_api_shadow
this is also a possible reason that a dev reported an issue with ci formatting

Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Gokul Nair <gnair@twitter.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api deps Approval required for changes to Envoy's external dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants