-
Notifications
You must be signed in to change notification settings - Fork 5.5k
server: add ProcessContext #7018
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
3d65c86
Add ProcessContext
ahedberg 5f35e5d
fix formatting
ahedberg 025e51a
Fix const-ness, add test
ahedberg 3cacee3
Pointers to refs, test cleanup
ahedberg 36fa5e1
Merge remote-tracking branch 'upstream/master' into process_context
ahedberg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| #pragma once | ||
|
|
||
| #include "envoy/common/pure.h" | ||
|
|
||
| namespace Envoy { | ||
|
|
||
| /** | ||
| * Represents some other part of the process. | ||
| */ | ||
| class ProcessObject { | ||
| public: | ||
| virtual ~ProcessObject() {} | ||
| }; | ||
|
|
||
| /** | ||
| * Context passed to filters to access resources from non-Envoy parts of the | ||
| * process. | ||
| */ | ||
| class ProcessContext { | ||
| public: | ||
| virtual ~ProcessContext() {} | ||
|
|
||
| /** | ||
| * @return the ProcessObject for this context. | ||
| */ | ||
| virtual const ProcessObject* get() const PURE; | ||
| }; | ||
|
|
||
| } // namespace Envoy | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| #pragma once | ||
|
|
||
| #include "envoy/server/process_context.h" | ||
|
|
||
| namespace Envoy { | ||
|
|
||
| class ProcessContextImpl : public ProcessContext { | ||
| public: | ||
| ProcessContextImpl(ProcessObject* process_object) : process_object_(process_object) {} | ||
|
ahedberg marked this conversation as resolved.
Outdated
|
||
| // ProcessContext | ||
| const ProcessObject* get() const override { return process_object_; } | ||
|
|
||
| private: | ||
| const ProcessObject* process_object_; | ||
| }; | ||
|
|
||
| } // namespace Envoy | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
If we're only ever going to support
get(), I guess it might be possible to eliminateProcessObjectand just pass in a subclass ofProcessContextwith the bits you are after. That said, I think it's good to have this indirection as we can grow it over time.One consideration I'm interested in your thoughts on. Let's say we have two filters and they want to get at different aspects of the state. Would it make sense to have a map from a string to the
ProcessObjects, or have them aware of the internals ofProcessObjectto do this? I'm thinking the former would make sense when you have multiple parties trying to collaborate onProcessObject, the latter when it's single party.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.
I had envisioned a map from string to ProcessObjects inside ProcessContext at some point, and then get() would do the lookup by that name. I didn't implement it here because we don't need it for our use case, and I wasn't sure if other folks would. I certainly could if we want to go with that design from the beginning, though.
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.
I think it's fine as is, the thing to be aware of then is in the future we might see an API breaking change. Let's ship and iterate.