Skip to content

Only generate a descriptor if all actions append an entry.#632

Merged
ccaraman merged 3 commits intomasterfrom
rl-descriptor
Mar 28, 2017
Merged

Only generate a descriptor if all actions append an entry.#632
ccaraman merged 3 commits intomasterfrom
rl-descriptor

Conversation

@ccaraman
Copy link
Contributor

When processing a rate limit configuration, all actions are to append a descriptor entry. If an action can't append an entry, there is no descriptor generated for the configuration.

be processed. For example given the following rate limit configuration, a request can
generate a few possible descriptors depending on what is present in the request.
If an action doesn't append a descriptor entry, no descriptor is generated for the configuration.
For example given the following rate limit configuration, there are two possible outcomes:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rephrase slightly. You don't actually show a second outcome. I would just move the clause after into this sentence, and show what gets output if a descriptor is output at all.

Also, this is from previous commit, but for the fixed with descriptor examples, it's a little strange to split the entries across multiple lines. If you want to line split, can you split on an entry basis and align the opening parenthesis. (Same elsewhere in this file).

* @param local_service_cluster supplies the name of the local service cluster.
* @param headers supplies the header for the request.
* @param remote_address supplies the trusted downstream address for the connection.
* @return true if the RateLimitAction populated the descriptor. Otherwise, false is returned.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Del "Otherwise, false is returned." (Obvious)

@ccaraman ccaraman merged commit 0359215 into master Mar 28, 2017
@mattklein123 mattklein123 deleted the rl-descriptor branch April 3, 2017 01:01
jplevyak added a commit to jplevyak/envoy that referenced this pull request Sep 9, 2020
…-upstream-wasm

Import envoyproxy and upstream wasm
mathetake added a commit that referenced this pull request Mar 3, 2026
**Commit Message**

This removes the inference extension related code from the controller to
reduce the size of the refactoring PR #629. We need to do the complete
redo on inference extension after #629, so this doesn't mean that we
drop the support for it.

---------

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants