-
Notifications
You must be signed in to change notification settings - Fork 5.3k
build: making admin optional #23903
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
build: making admin optional #23903
Changes from all commits
cf61641
e3f01ee
44549bc
eaec79b
a1d5c1a
dba9757
4a3f628
a5b0c5c
50c5aba
f15e6e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -279,6 +279,11 @@ class Admin { | |
| */ | ||
| static RequestPtr makeStaticTextRequest(absl::string_view response_text, Http::Code code); | ||
| static RequestPtr makeStaticTextRequest(Buffer::Instance& response_text, Http::Code code); | ||
|
|
||
| /** | ||
| * Closes the listening socket for the admin. | ||
| */ | ||
| virtual void closeSocket() PURE; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't seem to figure out why this new pure virtual method is needed. Oh is it because we're using
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we do, or the server (which now has an API pointer because it may not know about the impl class) can't close the socket.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe you, but I don't yet understand how to connect all the dots :) Why does the server need an API pointer now when before it had an impl pointer? |
||
| }; | ||
|
|
||
| } // namespace Server | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why does this need to be added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the tap filter has admin functionality. I didn't go through the exercise of carving that out, the entire tap filter simply won't compile.
Because of that we exempt tap here and add it back below as long as admin isn't disabled