Opened 13 months ago

Closed 13 months ago

Last modified 12 months ago

#28197 closed defect (fixed)

Rename some bandwidth file keys in the code

Reported by: juga Owned by: juga
Priority: Medium Milestone: sbws: 1.0.x-final
Component: Core Tor/sbws Version:
Severity: Normal Keywords: sbws-1.0-must-closed-moved-20181128
Cc: pastly, juga, teor Actual Points:
Parent ID: #28085 Points:
Reviewer: Sponsor:

Description

So that they have also that name in the bandwidth file spec when added.

Child Tickets

Change History (7)

comment:1 Changed 13 months ago by juga

I'd eliminate the parts that have _bs in their name, it was added to know that the unit was Bytes, but it's not clear from the name and if they unit is not specified the default should be Bytes.
The names of the keys after eliminating _bs are:
bw_median, bw_mean, desc_avg_bw, desc_obs_bw_last, desc_obs_bw_mean

comment:2 Changed 13 months ago by juga

The keys added by #28062 are having measured in their name.
I think that's confusing, because the scanner measured more than those relays, it's just they didn't pass some restrictions.
I'm not sure to_include is better name.

comment:3 Changed 13 months ago by teor

The full list of keys in sbws master is at:
https://github.com/torproject/sbws/blob/master/sbws/lib/v3bwfile.py#L29

When I worked as a database designer, we named fields using this pattern:

  • optional Adjective(s) (words that modify the thing)
  • Noun (what the thing is)
  • optional Adjectives (words that modify the format of the data)
  • Kind (what format the data is in)

We avoided most abbreviations, because they can mean different things. If we needed an abbreviation, we documented it, and used it consistently.

If we were designing this format again, here are the names I would suggest:

EXTRA_ARG_KEYVALUES:

  • software => software_name
  • software_version
  • file_created => file_creation_datetime
  • earliest_bandwidth => earliest_bandwidth_datetime
  • generator_started => generator_start_datetime

UNORDERED_KEYVALUES:

  • latest_bandwidth => latest_bandwidth_datetime

ALL_KEYVALUES:

  • version => format_version

STATS_KEYVALUES:

  • num_measured_relays => eligible_relay_count
  • num_target_relays => min_relay_count (documented abbreviation) or minimum_eligible_relay_count
  • num_net_relays => consensus_relay_count
  • perc_measured_relays => eligible_relay_percentage
  • perc_measured_targed => min_relay_percentage (documented abbreviation) or minimum_eligible_relay_percentage

BW_KEYVALUES_BASIC:

  • node_id
  • bw => bw_kps (documented abbreviations) or bandwidth_kilobytes_per_second

BW_KEYVALUES_FILE:

  • master_key_ed25519 => ed25519_key or ed25519_master_key
  • nick => nick_text (documented abbreviation) or nickname_text
  • rtt => rtt_milliseconds (documented abbreviation) or round_trip_time_milliseconds
  • time => latest_relay_bandwidth_datetime
  • success => bandwidth_success_count
  • error_stream => bandwidth_stream_error_count
  • error_circ => bandwidth_circuit_error_count
  • error_misc => bandwidth_other_error_count

BW_KEYVALUES_EXTRA_BWS:

  • bw_bs_median => median_bw_bps (documented abbreviations) or median_bandwidth_bytes_per_second
  • bw_bs_mean => mean_bw_bps (documented abbreviations) or mean_bandwidth_bytes_per_second
  • desc_avg_bw_bs => desc_avg_bw_limit_bps (documented abbreviations) or descriptor_average_bandwidth_limit_bytes_per_second
  • desc_obs_bw_bs_last => latest_desc_obs_bw_bps (documented abbreviations) or latest_descriptor_observed_bandwidth_bytes_per_second
  • desc_obs_bw_bs_mean => mean_desc_obs_bw_bps (documented abbreviations) or mean_descriptor_observed_bandwidth_bytes_per_second

You can decide if you want to follow this pattern. You can change the names I suggested. You can leave some names as they are.

If you rename a field in the 1.1.0 spec, it is a breaking change, and you should update the spec version to 2.0.0. (I don't think breaking changes are a good use of your time.)

If you add fields to the 1.1.0 spec, they are new features, and you should update the spec version to 1.2.0.

Version 0, edited 13 months ago by teor (next)

comment:4 in reply to:  3 Changed 13 months ago by juga

Replying to teor:
[...]

When I worked as a database designer, we named fields using this pattern:

  • optional Adjective(s) (words that modify the thing)
  • Noun (what the thing is)
  • optional Adjectives (words that modify the format of the data)
  • Kind (what format the data is in)

It's a similar pattern i was trying to follow. The main differences being:

  • in all the other humans languages i know, adjetives come after noun, so even if the key names are in English, my way of thinking gives more importance to the noum.
  • i'd have thought that units (Kind) comes at the end, except that after naming the keys with only nouns and units, i wanted other keys with adjetives to start with that noun + unit + adjetive. This was probably a bad choice, since i'd still put _ls for list or other "Kind"s at the end.

comment:5 Changed 13 months ago by juga

Status: assignedneeds_review

comment:6 Changed 13 months ago by juga

Resolution: fixed
Status: needs_reviewclosed

Merged

comment:7 Changed 12 months ago by teor

Keywords: sbws-1.0-must-closed-moved-20181128 added
Milestone: sbws 1.0 (MVP must)sbws: 1.0.x-final

Move all closed sbws 1.0 must tickets to sbws 1.0.x-final

Note: See TracTickets for help on using tickets.