Skip to content

unhide custom settings option in protocol options#22376

Closed
ramaraochavali wants to merge 1 commit intoenvoyproxy:mainfrom
ramaraochavali:fix/unhide_settings
Closed

unhide custom settings option in protocol options#22376
ramaraochavali wants to merge 1 commit intoenvoyproxy:mainfrom
ramaraochavali:fix/unhide_settings

Conversation

@ramaraochavali
Copy link
Copy Markdown
Contributor

This has been implemented by #9964
Commit Message:unhide custom settings option
Additional Description:unhide custom settings option in protocol options
Risk Level:N/A
Testing:N/A
Docs Changes:N/A
Release Notes:N/A
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @mattklein123
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #22376 was opened by ramaraochavali.

see: more, trace.

@ramaraochavali
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

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

see: more, trace.

@zuercher zuercher assigned lizan and unassigned mattklein123 Jul 26, 2022
@zuercher
Copy link
Copy Markdown
Member

Reassigned @lizan as api shepherd

@lizan
Copy link
Copy Markdown
Member

lizan commented Jul 26, 2022

I don't have the original PR context, @alyssawilk did the deprecation PR described https://github.com/envoyproxy/envoy/pull/9964/files#r394441848 happen?

@alyssawilk
Copy link
Copy Markdown
Contributor

allow_connect doesn't look deprecated so I'm going with "no"
I think to unhide this, we should deprecate allow_connect, and config-check that if custom_settings_parameters is used, that allow_connect is not true, and update the comments for this field as well as relevant docs.

@alyssawilk alyssawilk self-assigned this Jul 26, 2022
@ramaraochavali
Copy link
Copy Markdown
Contributor Author

Oh. I did not know the dependency on allow_connect. Will close this PR.

@ramaraochavali ramaraochavali deleted the fix/unhide_settings branch July 27, 2022 04:07
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.

5 participants