EngineBuilder API: addDnsQueryTimeoutSeconds#1583
Merged
Conversation
added 2 commits
July 9, 2021 16:41
Signed-off-by: Jose Nino <jnino@lyft.com>
Merged
junr03
commented
Jul 10, 2021
|
|
||
| private struct TestFilter: Filter {} | ||
|
|
||
| // swiftlint:disable:next type_body_length |
Member
Author
There was a problem hiding this comment.
not sure if this is the kosher way of doing this @rebello95
Contributor
There was a problem hiding this comment.
this is fine, or honestly we could just remove this rule from .swiftlint.yml
Contributor
There was a problem hiding this comment.
the "fix" would be to split this file up
added 5 commits
July 21, 2021 16:19
Signed-off-by: Jose Nino <jnino@lyft.com>
Member
Author
|
@rebello95 @buildbreaker ready for review |
rebello95
previously approved these changes
Jul 23, 2021
| * @param dnsRefreshSeconds rate in seconds to refresh DNS. | ||
| * @param dnsFailureRefreshSecondsBase base rate in seconds to refresh DNS on failure. | ||
| * @param dnsFailureRefreshSecondsMax max rate in seconds to refresh DNS on failure. | ||
| * @param dnsQueryTimeoutSeconds rate in seconds to timeout DNS queries. |
added 2 commits
July 26, 2021 11:25
Signed-off-by: Jose Nino <jnino@lyft.com>
Member
Author
|
@buildbreaker can I get a review to re-approve @rebello95's dismissed review? |
buildbreaker
approved these changes
Jul 27, 2021
buildbreaker
left a comment
There was a problem hiding this comment.
I've never been a fan of file length linters (we do this at Lyft's iOS code base actually)
Maybe we can add a task to remove that lint check?
Reference comment: https://github.com/envoyproxy/envoy-mobile/pull/1583/files#r667261030
Augustyniak
pushed a commit
to Augustyniak/envoy-mobile
that referenced
this pull request
Aug 2, 2021
Description: API for addDnsQueryTimeoutSeconds to configure behavior introduced in envoyproxy#1580 Risk Level: low - optional builder API Testing: updated suite. Docs Changes: added Signed-off-by: Jose Nino <jnino@lyft.com> Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description: API for addDnsQueryTimeoutSeconds to configure behavior introduced in #1580
Risk Level: low - optional builder API
Testing: updated suite.
Docs Changes: added
Signed-off-by: Jose Nino jnino@lyft.com