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

unhide custom settings option in protocol options #22376

Closed

Conversation

ramaraochavali
Copy link
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

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
Contributor Author

/retest

@repokitteh-read-only
Copy link

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
Member

Reassigned @lizan as api shepherd

@lizan
Copy link
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
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
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