Opened 9 months ago

Closed 9 months ago

Last modified 4 months ago

#28600 closed defect (wontfix)

Refactor: store the minimum of the bandwidth values from the consenus in the results

Reported by: juga Owned by:
Priority: Medium Milestone: sbws: 1.0.x-final
Component: Core Tor/sbws Version:
Severity: Normal Keywords:
Cc: juga Actual Points:
Parent ID: #25925 Points:
Reviewer: Sponsor:

Description

Currently we store the descriptor and network status bandwidth values in the raw results to be able to calculate the minimum from them in the bandwidth file.
In case we don't need to these values for anything else, we could calculate and store the minimum of them in the bandwidth raw results.

Child Tickets

Change History (8)

comment:1 Changed 9 months ago by juga

Parent ID: #25925

comment:2 Changed 9 months ago by starlight

memory is cheap, current and future developer time expensive

comment:3 in reply to:  description Changed 9 months ago by teor

Replying to juga:

Currently we store the descriptor and network status bandwidth values in the raw results to be able to calculate the minimum from them in the bandwidth file.
In case we don't need to these values for anything else, we could calculate and store the minimum of them in the bandwidth raw results.

The bug #28588 happened because torflow did a calculation and stored one value.
We can avoid future bugs like #28588 by taking the minimum as late as possible.

I think we should not do this refactor.
What do you think?

comment:4 Changed 9 months ago by juga

Yes, i'll do the refactor

comment:5 Changed 9 months ago by teor

How does the refactor make the code easier to understand?

The torflow code was really hard to understand, because the min() was hidden in pytorctl.
Now you want to add the same problem to sbws. I don't understand why.

comment:6 Changed 9 months ago by juga

pytorctl is a different repo that torflow, relaylist and generate are in the same one.
Also, the name of the variable in pytorctl didn't correspond to what it was doing.
If relaylist would have just one bandwidth attribute or method called something like minimum_descriptor_consensus_bandwidth, it would be more obvious.
However, i've thinking about other refactoring to don't have to repeat all the bandwidth variables in relaylist, resultdump and v3bwfile, i'll open other ticket and explain it there.

comment:7 Changed 9 months ago by juga

Resolution: wontfix
Status: newclosed

We can then close this

comment:8 Changed 4 months ago by teor

Milestone: sbws: 1.0.x-final

Moving closed sbws tickets to sbws: 1.0.x-final.

Note: See TracTickets for help on using tickets.