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.

Sign up for GitHub

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

Jump to bottom

[http] Remove legacy codecs #14381

Merged
merged 6 commits into from Dec 16, 2020
Merged

[http] Remove legacy codecs #14381

merged 6 commits into from Dec 16, 2020

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Dec 11, 2020 •
edited

Signed-off-by: Asra Ali asraa@google.com

Commit Message: Removes legacy codecs.
Risk Level: Low, already default.
Testing: All existing tests remain the same, no functional changes.
Release Notes: added notice runtime flag and legacy code paths were removed.
Fixes: #10878

@asraa
remove legacy codecs
9b302c8 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
fix flood integration test
99ad59c 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
fix frame replay
7011f4c 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
fix config test
6551049 
Signed-off-by: Asra Ali <asraa@google.com>
@yanavlasov
Copy link
Contributor

LGTM. I think clang-tidy is pre-existing

@asraa asraa mentioned this pull request Dec 14, 2020
Closed
@asraa asraa marked this pull request as ready for review December 14, 2020 16:45
@asraa
satisfy clang tidy
d9cec9e 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
Copy link
Contributor Author

asraa commented Dec 15, 2020

Still getting a clang-tidy error despite //source/common/event:real_time_system_lib appearing in the deps of test_time_lib. I believe this is a flake/clang-tidy mistake? People have seen this before, not sure why. @lizan

Diagnostics:
- DiagnosticMessage:
    FileOffset: 23
    FilePath: ./test/test_common/test_time.h
    Message: '''common/event/real_time_system.h'' file not found'
    Replacements: []
  DiagnosticName: clang-diagnostic-error

@yanavlasov
Copy link
Contributor

This looks like fixing the issue.

diff --git a/test/common/http/http2/BUILD b/test/common/http/http2/BUILD
index 8d8d149d1..8dca77e67 100644
--- a/test/common/http/http2/BUILD
+++ b/test/common/http/http2/BUILD
@@ -47,6 +47,7 @@ envoy_cc_test_library(
     external_deps = ["abseil_optional"],
     deps = [
         "//source/common/http/http2:codec_lib",
+        "//test/mocks:common_lib",
     ],
 )

@asraa
fix clang tidy
20541db 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
Copy link
Contributor Author

asraa commented Dec 15, 2020

Wow, thanks -- I should have scrolled more through the logs to see the pointer to test util. Fixed.

@yanavlasov yanavlasov merged commit 09134ff into envoyproxy:master Dec 16, 2020
@mattklein123
Copy link
Member

Congratulations @asraa!!! Very nicely done. 🎉

@asraa
Copy link
Contributor Author

asraa commented Dec 16, 2020

Lots of code removed! Hope it goes smooth :)

mpuncel added a commit to mpuncel/envoy that referenced this pull request Dec 16, 2020
@mpuncel
Merge branch 'master' into mpuncel/http2-hc-goaway
d15bbf3 
* master: (49 commits)
  sds: allow multiple init managers share sds target (envoyproxy#14357)
  [http] Remove legacy codecs (envoyproxy#14381)
  http2: Add integration tests for METADATA and RST_STREAM frame flood mitigation for upstream servers (envoyproxy#14365)
  test: start dissolving :printers_include rule. (envoyproxy#14429)
  integration tests: re-enable set_node_on_first_message_only (envoyproxy#14270)
  formatter: add a formatter that returns a google::protobuf::Struct rather than a string (envoyproxy#14258)
  ratelimit: support returning custom response bodies for non-OK responses from the external ratelimit service (envoyproxy#14189)
  deps: update protobuf to 3.14 (envoyproxy#14253)
  stream_info: add setResponseCode and update local_reply to take a normal StreamInfo (envoyproxy#14402)
  http: alpn upstream (envoyproxy#13922)
  Moved starttls integration test to test/extensions/transport_sockets/starttls. (envoyproxy#14425)
  generic conn pool: directly use thread local cluster (envoyproxy#14423)
  wasm: add mathetake to CODEOWNERS (envoyproxy#14427)
  wasm: clear route cache when modifying HTTP request headers. (envoyproxy#14318)
  tls: disable TLS inspector injection (envoyproxy#14404)
  aggregate cluster: cleanups (envoyproxy#14411)
  Mark starttls_integration_test flaky on Windows (envoyproxy#14419)
  tcp: improved unit testing (envoyproxy#14415)
  config: making protocol config explicit (envoyproxy#14362)
  wasm: dead code (envoyproxy#14407)
  ...

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yanavlasov yanavlasov yanavlasov approved these changes

@alyssawilk alyssawilk Awaiting requested review from alyssawilk

@mattklein123 mattklein123 Awaiting requested review from mattklein123

Assignees

@yanavlasov yanavlasov

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[HTTP] High level design: Remove exceptions in codecs
3 participants
@asraa @yanavlasov @mattklein123