add user agent and use case info to faucet json body #572
Conversation
| @@ -31,6 +31,7 @@ async def generate_faucet_wallet( | |||
| wallet: Optional[Wallet] = None, | |||
| debug: bool = False, | |||
| faucet_host: Optional[str] = None, | |||
| use_case: Optional[str] = None, | |||
There was a problem hiding this comment.
Why not just call this param user_agent? use_case is a very confusing name IMO
There was a problem hiding this comment.
We suggested that it be moved to json instead of http header
There was a problem hiding this comment.
user_agent is specific to xrpl.js - use-case is pointing towards things like: workshop, int test, unit tests..
There was a problem hiding this comment.
Should there be a follow up PR which sets
use_case="integration_test"oruse_case="unit_test"?
I didn't see a unit test that call the fundWallet function
There was a problem hiding this comment.
Should there be a follow up PR which sets
use_case="integration_test"oruse_case="unit_test"?I didn't see a unit test that call the fundWallet function
No but generate_faucet_wallet is called in places. That should allow for you pass use case.
There was a problem hiding this comment.
user_agent is specific to xrpl.js - use-case is pointing towards things like: workshop, int test, unit tests..
I'm a little confused as to what this means.
Personally I don't think use_case is a very descriptive name and I would have no idea what that meant if I saw it in the docs. Maybe something along the lines of source would be more descriptive.
|
Should there be a follow up PR which sets |
jonathanlei
changed the title
jonathanlei
changed the title
|
@jonathanlei pinging about adding use_case to generate_faucet_wallet |
|
@jonathanlei conflicts after latest master |
High Level Overview of Change
aadd user agent and use case info to faucet transactions for analytics
Context of Change
This is part of the testnet analytics effort.
Type of Change
[ X] New feature (non-breaking change which adds functionality)