Opened 2 years ago

Closed 8 months ago

#16552 closed enhancement (fixed)

Don't require searches by IPv6 address to start with [

Reported by: karsten Owned by:
Priority: Very Low Milestone: Onionoo-1.2.0
Component: Metrics/Onionoo Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Searches by IPv6 address need to include [], and partial searches need to start with [. Example: "[2001:858:2:2:aabb:0:563b:1526]". That's not as intuitive as it could be. We should consider making the [] part optional for IPv6 searches. Not sure if that's a good idea, but I can't come up with major arguments against it.

This enhancement request originates from #10128 and has low priority.

Child Tickets

Change History (7)

comment:1 Changed 10 months ago by karsten

Severity: Normal
Status: newneeds_review

I still didn't come up with reasons against making this change, so I wrote a patch.

The only catch is that we're changing the semantics of the current search parameter, because a search for, say "2001", might now return even more relays: matching hex fingerprint starts, matching base64 fingerprint starts, matching nickname parts, and matching IPv6 address starts. Admitted, it's a tiny change, but it's a change. That's why we should do the major-protocol-update dance where we announce a change 1 month in advance, set next_major_version_scheduled, and merge 1 month later. But before we start that dance, let's get the patch ready for merge.

Please review my task-16552 branch.

comment:2 Changed 9 months ago by iwakeh

Status: needs_reviewmerge_ready

Looks fine and has tests :-)

So, the reason for raising the major protocol number is that a search for 0815 could now return all relays having ipv6 addresses starting with the search string?
Clients supporting 3.1 shouldn't break, b/c of this change. In a way I would count loosening the restriction on searching for ipv6 addresses to adding a new fields. Thus, this could be in 3.2-1.1.0?

Merge ready, but I would say this change doesn't warrant protocol version 4.0.

comment:3 Changed 9 months ago by karsten

Thanks for looking! :)

But not raising the protocol version to 4.0 feels wrong. We are changing the most important parameter by returning potentially more results than previously. That won't break clients, but it might confuse users. In a way, protocol changes leading to 2.0 and to 3.0 were comparable to this one: in both cases we returned potentially more results than before. Those are major protocol changes, because clients either need to adapt or decide they don't care. That's different from minor protocol version updates which clients can ignore if they wish without even looking.

I'll keep this as merge_ready until we tag the next release, in which case I'll commit a placeholder for version 4.0 and set next_major_version_scheduled to 1 month later.

comment:4 Changed 9 months ago by karsten

Milestone: Onionoo 3.2-1.1.0

Assigning to the next milestone, so that we don't forget to start dancing when we put out that release.

comment:5 Changed 9 months ago by iwakeh

Milestone: Onionoo 3.2-1.1.0Onionoo-1.2.0

comment:6 Changed 8 months ago by karsten

Status: merge_readyneeds_review

I just rebased this branch to current master and prepared the release that is scheduled for next Tuesday. The new branch is task-16552-3. Feel free to review that branch once more before I merge it next Tuesday, but I'll also ask you to review the release before it goes out. Thanks!

comment:7 Changed 8 months ago by karsten

Resolution: fixed
Status: needs_reviewclosed

Pushed to master, now preparing the release for later today. Closing. Thanks!

Note: See TracTickets for help on using tickets.