Skip to content

config: Deprecate opaque Struct config, again.#8472

Merged
lizan merged 16 commits intoenvoyproxy:masterfrom
yanavlasov:deprecate-struct-v2
Oct 26, 2019
Merged

config: Deprecate opaque Struct config, again.#8472
lizan merged 16 commits intoenvoyproxy:masterfrom
yanavlasov:deprecate-struct-v2

Conversation

@yanavlasov
Copy link
Contributor

@yanavlasov yanavlasov commented Oct 3, 2019

Deprecate opaque Struct config, again.
Fix unit tests to use the google.protobuf.Any

Reverts PR #6346
Fixes #8403

Risk Level: Low
Testing: Unit Tests
Docs Changes: N/A
Release Notes: deprecation notice

Signed-off-by: Yan Avlasov yavlasov@google.com

Fix unit tests to use the google.protobuf.Any

Signed-off-by: Yan Avlasov <yavlasov@google.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #8472 was opened by yanavlasov.

see: more, trace.

@yanavlasov yanavlasov changed the title Deprecate opaque Struct config, again. config: Deprecate opaque Struct config, again. Oct 3, 2019
@yanavlasov yanavlasov marked this pull request as ready for review October 3, 2019 15:27
@yanavlasov
Copy link
Contributor Author

@lizan , @htuch please take a look at the initial set of changes. I still need to fix two unit tests with --define deprecated_features=disabled and one without.
Let me know if this is moving in the right direction.

…id configuration uses the Any type.

Signed-off-by: Yan Avlasov <yavlasov@google.com>
@lizan
Copy link
Member

lizan commented Oct 3, 2019

Thanks @yanavlasov, this looks great and is definitely in right direction! You also need to run the proto format check fix to reflect in changes v3alpha and update deprecated doc.

Signed-off-by: Yan Avlasov <yavlasov@google.com>
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.

Awesome.
/wait

…/extensions/filters/network/thrift_proxy:config_test tests

Signed-off-by: Yan Avlasov <yavlasov@google.com>
Signed-off-by: Yan Avlasov <yavlasov@google.com>
Signed-off-by: Yan Avlasov <yavlasov@google.com>
@yanavlasov
Copy link
Contributor Author

All tests pass (for now). I've updated docs/root/intro/deprecated.rst Let me know if there is anything else that needs to be done in this PR.
I will work on parsing TypedStruct in config protos next.

@htuch
Copy link
Member

htuch commented Oct 7, 2019

@yanavlasov looks great, thanks; can we review/merge the PR for TypedStruct first? Usually I would be OK with knowing they are coming at roughly the same time, but we have a lot of other churn with API and we previously had to undeprecate because we didn't have a known solution landed for Any, so treading a bit more carefully this time around.

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.

Couple of test comments..

Signed-off-by: Yan Avlasov <yavlasov@google.com>
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.

Awesome, this is such an epic PR. Thanks. LGTM modulo two comments.
/wait

@stale
Copy link

stale bot commented Oct 18, 2019

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Oct 18, 2019
Signed-off-by: Yan Avlasov <yavlasov@google.com>
Signed-off-by: Yan Avlasov <yavlasov@google.com>
@stale stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Oct 22, 2019
htuch
htuch previously approved these changes Oct 24, 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.

Awesome, LGTM, Can you merge master and then we can ship?
/wait

Signed-off-by: Yan Avlasov <yavlasov@google.com>
Signed-off-by: Yan Avlasov <yavlasov@google.com>
@lizan lizan merged commit 2d7e2bb into envoyproxy:master Oct 26, 2019
@yanavlasov yanavlasov deleted the deprecate-struct-v2 branch February 1, 2021 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

re-deprecate opaque Struct config

3 participants