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

Implement customizable timeouts for calls #22

Merged
merged 1 commit into from
Sep 19, 2018

Conversation

andrewvy
Copy link
Owner

@andrewvy andrewvy commented Nov 6, 2017

Implements #16.

New option called :timeout will be used (default 30s) for synchronous calls.

ChromeRemoteInterface.RPC.Page.printToPDF(page_pid, %{}, timeout: 60_000)

@garrinmf
Copy link

Is there anything stopping this from getting merged? I was getting ready to implement the same thing for my own needs.

@andrewvy
Copy link
Owner Author

Hey @garrinmf! Thanks for the ping, this PR wasn't merged in as the timeout functionality wasn't needed for my immediate purposes. I instead opted for the async option.

Though, I am totally happy to merge this functionality if it's needed. :)

@bcardarella
Copy link
Contributor

@andrewvy we have a need for the timeout option. However, I believe the default should be the default GenServer timeout of 5_000 instead of the 30_000 in the PR.

Use case: we are generating PDFs. Some of them are taking more than 5 seconds to generate.

@garrinmf
Copy link

For my use case I need the timeout and blocking, not that I couldn't recreate it a level up but it'd be nice if it was baked in. And I'd concur that 5_000 probably makes more sense as a default.

@andrewvy andrewvy merged commit a5e524a into master Sep 19, 2018
@andrewvy andrewvy deleted the feature/custom-timeouts branch September 19, 2018 17:30
@andrewvy andrewvy mentioned this pull request Sep 19, 2018
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.

None yet

3 participants