tools: adding a python script for making deprecated featured fail-by-default #5922
Conversation
…default Signed-off-by: Alyssa Wilk <[email protected]>
|
Apologies to whoever reviews this: I don't really do python. I figured if I tried to submit the original .sh version someone was going to make me rewrite it in Python anyway :-P |
alyssawilk
force-pushed
the
deprecation_script
branch
from
3dfcd56
to
d024f6b
Compare
Signed-off-by: Alyssa Wilk <[email protected]>
alyssawilk
force-pushed
the
deprecation_script
branch
from
d024f6b
to
e345914
Compare
There was a problem hiding this comment.
So, I think the really clean way to do this is to add a proto annotation deprecated_at_version, containing the release version at which a field was deprecated. Then we can automatically build a runtime_features_deprecated_flags.h file via genrule during the build build via a protoc plugin.
This is a fair bit of work to do right, so maybe you can file an issue and we can start with what you have here.
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.STDOUT, | ||
| shell=True) | ||
| grep_output = grep.communicate()[0] |
There was a problem hiding this comment.
Prefer subprocess.check_output(..)
|
|
||
| # Now discard any deprecated features already listed in runtime_features | ||
| exiting_deprecated_regex = re.compile(r".*\"envoy.deprecated_features.(.*):(.*)\",.*") | ||
| features = open('source/common/runtime/runtime_features.h', "r") |
There was a problem hiding this comment.
Nit: prefer context managers:
with open(...) as f:
do stuff with f
this auto-closes.
| import fileinput | ||
|
|
||
| grep = subprocess.Popen( | ||
| 'grep -r "deprecated = true" api/*', |
There was a problem hiding this comment.
Hmm, this is pretty fuzzy; the clean way to do it would be via Python reflection and protobuf, or with a protoc plugin. It might be good enough though.
There was a problem hiding this comment.
Yeah, for a script we are literally running once a quarter, I think the hacky way is fine :-)
We're also going to want to do a similar thing for runtime feature flags, and I'd prefer we stick everything in one script. If we start simple on the flags side we will probably want to grep and dedup there as well.
We can easily note deprecation in with a comment in runtime_features.h if we think it's worth tracking, just by running the script with the version as we do with deprecate_version/...
Signed-off-by: Alyssa Wilk <[email protected]>
Signed-off-by: Alyssa Wilk <[email protected]>
| if not raw_input('Apply runtime changes? [yN] ').strip().lower() in ("y", "yes"): | ||
| exit(1) | ||
|
|
||
| for line in fileinput.FileInput('source/common/runtime/runtime_features.h', inplace=1): |
There was a problem hiding this comment.
Nit: can you be consistent in whether to use single or double quotes for strings across this file?
Signed-off-by: Alyssa Wilk <[email protected]>
…default (envoyproxy#5922) plus venv wrappers shamelessly cut-and-pasted from tools/deprecate_version Risk Level: n/a (tools only) Testing: manual testing Docs Changes: updated GOVERNANCE.md Release Notes: n/a Fixes envoyproxy#5559 Signed-off-by: Alyssa Wilk <[email protected]> Signed-off-by: Fred Douglas <[email protected]>
plus venv wrappers shamelessly cut-and-pasted from tools/deprecate_version
Risk Level: n/a (tools only)
Testing: manual testing
Docs Changes: updated GOVERNANCE.md
Release Notes: n/a
Fixes #5559