Skip to content
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

Decrease Wingman timeout from 3.3 minutes to 2 seconds (configurable) #1688

Merged
merged 7 commits into from
Apr 8, 2021

Conversation

isovector
Copy link
Collaborator

At some point I made a boneheaded move, thinking that 1e8 was the number of microseconds in a second. I'm not sure where this number came from, but it's 2 orders of magnitude too large. As a result, the timeout feature in Wingman didn't work very well.

This PR changes the tactic timeout to be a more reasonable 2 seconds, down from 3+ minutes.

@pepeiborra
Copy link
Collaborator

Why 2 seconds though. Seems a bit short. Make it configurable ?

@isovector isovector changed the title Decrease Wingman timeout from 3.3 minutes to 2 seconds Decrease Wingman timeout from 3.3 minutes to 2 seconds (configurable) Apr 7, 2021
@isovector
Copy link
Collaborator Author

Now officially configurable!

Copy link
Member

@Ailrun Ailrun left a comment

Choose a reason for hiding this comment

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

LGTM

-- semigroup
, "sconcat", "stimes"
-- monoid
, "mconcat"
Copy link
Member

Choose a reason for hiding this comment

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

Why are these changes bundled together?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm an absolute madman

@isovector isovector added the merge me Label to trigger pull request merge label Apr 8, 2021
@mergify mergify bot merged commit d5c5874 into haskell:master Apr 8, 2021
@ndmitchell
Copy link
Collaborator

My rule is always make timeouts in seconds, and Double (or even better, the Seconds alias https://hackage.haskell.org/package/extra-1.7.9/docs/Extra.html#t:Seconds). Then also use the timeout from Extra (https://hackage.haskell.org/package/extra-1.7.9/docs/Extra.html#v:timeout) and everything works properly. Having timeouts in milliseconds, microseconds or whatever is just too confusing.

@isovector isovector deleted the short-timeout branch May 21, 2021 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants