-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Fix default timeouts for Marathon provider. #1398
Conversation
Marking WIP because I still want to add an integration test. |
b7b166f
to
788b661
Compare
ce23287
to
717cf17
Compare
I went down a huge rabbit hole trying to add an integration test. I decided to carve out the test into a separate PR #1406 and reduced this one. @emilevauge can we approve the PR without the extended test for now? I'd like the fix to be in first, will continue to work on the test. |
Ping @emilevauge |
717cf17
to
bededab
Compare
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.
LGTM
a9594e2
to
360e8e1
Compare
The timeouts were given without a unit, which caused nanoseconds scale to be applied when we switched the type from int to flaeg.Duration.
LGTM |
The timeouts were given without a unit, which caused nanoseconds scale to be applied when we switched the type from
int
toflaeg.Duration
.Fixes a regression introduced by #1350.
Discovered along #1390.