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

add timeout to faucet #518

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dangell7
Copy link
Contributor

High Level Overview of Change

My faucet is slow and this PR adds a timeout to the faucet post request.

Context of Change

I used a default timeout of 5 however there are some sub-settings we could add if we wanted to get more fine grained with the timeout.

connect: float | UnsetType | None = UNSET,
read: float | UnsetType | None = UNSET,
write: float | UnsetType | None = UNSET,

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

Test Plan

@jonathanlei
Copy link
Contributor

I am actually working on adding a memos params (with default values) to all faucet transactions in client libraries, this PR is highly related, would you mind if I base my PR on yours? thanks

@dangell7
Copy link
Contributor Author

I am actually working on adding a memos params (with default values) to all faucet transactions in client libraries, this PR is highly related, would you mind if I base my PR on yours? thanks

Not at all :)

@JST5000
Copy link
Contributor

JST5000 commented May 24, 2023

This didn't end up being added as part of the memos change - @dangell7 I think this would make sense as an optional parameter, but not as a default timeout value (because I'm not sure other users want a timeout, and picking a number that works for everyone with sidechain setups seems hard to do).

If you don't want to add that, we can close this potentially.

@mvadari
Copy link
Collaborator

mvadari commented Jun 8, 2023

@dangell7 FYI the Ripple faucet repo is public if you'd rather use that (it might be faster)
https://github.com/xpring-eng/testnet-faucet

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

4 participants