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
EngineBuilder API: addDnsQueryTimeoutSeconds #1583
Conversation
Signed-off-by: Jose Nino <jnino@lyft.com>
@@ -19,6 +19,7 @@ fixture_template: | |||
|
|||
private struct TestFilter: Filter {} | |||
|
|||
// swiftlint:disable:next type_body_length |
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.
not sure if this is the kosher way of doing this @rebello95
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.
this is fine, or honestly we could just remove this rule from .swiftlint.yml
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 "fix" would be to split this file up
Signed-off-by: Jose Nino <jnino@lyft.com>
@rebello95 @buildbreaker ready for review |
@@ -38,6 +39,7 @@ | |||
* @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. |
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.
Nit: indentation
Signed-off-by: Jose Nino <jnino@lyft.com>
@buildbreaker can I get a review to re-approve @rebello95's dismissed review? |
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'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
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>
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