Implement support for I2CP options #415
Conversation
Codecov Report
@@ Coverage Diff @@
## master #415 +/- ##
=========================================
- Coverage 88.01% 88% -0.02%
=========================================
Files 146 146
Lines 27578 27592 +14
Branches 3921 3924 +3
=========================================
+ Hits 24274 24282 +8
- Misses 2599 2604 +5
- Partials 705 706 +1
Continue to review full report at Codecov.
|
|
|
||
| def unescape(val): | ||
| return val.replace('\ESCAPED-COMMA', ',').replace('\ESCAPED-COLON', ':') | ||
|
|
There was a problem hiding this comment.
This all probably deserves a comment, like why is this needed? It was also surprising to me to learn that '\,' is the very same as r'\,' in this case -- but it might be better to use the r (raw) prefixed strings anyway, so the reader knows you meant that to be "backslash comma" not just "comma".
There was a problem hiding this comment.
yeah, especially because the existing i2p_provider puts backslashes into escaped_sam_port. Maybe we could have a few examples of what the original values of sam_port and i2cp.options are, and what gets written into the config file? And some clear rules about what is legal in each kind of string and what is not.
|
Besides the "surprising string things", this looks good to me; @warner? Is it worth trying to choose an options separator that the Twisted endpoint-strings didn't already 'steal' (namely, "something besides colon and comma")? |
| i2cp_options = cli_config["i2p-i2cp-options"] | ||
| if i2cp_options: | ||
| tahoe_config_i2p["i2cp.options"] = i2cp_options | ||
| escaped_options = i2cp_options.replace(':', '\:') |
There was a problem hiding this comment.
is this missing a line to replace(',', '\,') ? The inverse is done below.
|
other than the missing |
|
@str4d , I think this is in your court now |
|
In yesterday's devchat, with @str4d 's help, we decided on a different approach for this, so we're WONTFIXing this in favor of the scheme documented in https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2889 |
|
The ticket-2889 config changes have landed, so this PR can now be rewritten in terms of the i2p_provider directly creating the Endpoint object, when tahoe.cfg uses |
See https://geti2p.net/en/docs/protocol/i2cp for supported options.