This repository has been archived by the owner on May 4, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 12
Ticket30406 refactor header constants #357
Open
juga0
wants to merge
6
commits into
torproject:master
Choose a base branch
from
juga0:ticket30406_refactor_header_constants
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
8a5efb1
chg: Add stem's bandwidth_file.py part
juga0 ebbb9ff
chg: v3bwfile: !refactor. Replace constants
juga0 c2dfe26
fixup! chg: Add stem's bandwidth_file.py part
juga0 2d92fbd
fixup! chg: v3bwfile: !refactor. Replace constants
juga0 9214877
new: tests: Add test network data and v3bwfile test
juga0 c3769b9
fixup! new: tests: Add test network data and v3bwfile test
juga0 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
"""Stem's Bandwidth file parser part. | ||
To be removed on Stem's release 1.8.0.""" | ||
|
||
# Converts header attributes to a given type. Malformed fields should be | ||
# ignored according to the spec. | ||
|
||
def _str(val): | ||
return val # already a str | ||
|
||
|
||
def _int(val): | ||
return int(val) if (val and val.isdigit()) else None | ||
|
||
|
||
def _date(val): | ||
try: | ||
return stem.util.str_tools._parse_iso_timestamp(val) | ||
except ValueError: | ||
return None # not an iso formatted date | ||
|
||
|
||
def _csv(val): | ||
return map(lambda v: v.strip(), val.split(',')) if val is not None else None | ||
|
||
|
||
# mapping of attributes => (header, type) | ||
|
||
HEADER_ATTR = { | ||
# version 1.1.0 introduced headers | ||
|
||
'version': ('version', _str), | ||
|
||
'software': ('software', _str), | ||
'software_version': ('software_version', _str), | ||
|
||
'earliest_bandwidth': ('earliest_bandwidth', _date), | ||
'latest_bandwidth': ('latest_bandwidth', _date), | ||
'created_at': ('file_created', _date), | ||
'generated_at': ('generator_started', _date), | ||
|
||
# version 1.2.0 additions | ||
|
||
'consensus_size': ('number_consensus_relays', _int), | ||
'eligible_count': ('number_eligible_relays', _int), | ||
'eligible_percent': ('percent_eligible_relays', _int), | ||
'min_count': ('minimum_number_eligible_relays', _int), | ||
'min_percent': ('minimum_percent_eligible_relays', _int), | ||
|
||
# version 1.3.0 additions | ||
|
||
'scanner_country': ('scanner_country', _str), | ||
'destinations_countries': ('destinations_countries', _csv), | ||
|
||
# version 1.4.0 additions | ||
|
||
'time_to_report_half_network': ('time_to_report_half_network', _int), | ||
|
||
'recent_stats.consensus_count': ('recent_consensus_count', _int), | ||
'recent_stats.prioritized_relay_lists': ('recent_priority_list_count', _int), | ||
'recent_stats.prioritized_relays': ('recent_priority_relay_count', _int), | ||
'recent_stats.measurement_attempts': ('recent_measurement_attempt_count', _int), | ||
'recent_stats.measurement_failures': ('recent_measurement_failure_count', _int), | ||
'recent_stats.relay_failures.no_measurement': ('recent_measurements_excluded_error_count', _int), | ||
'recent_stats.relay_failures.insuffient_period': ('recent_measurements_excluded_near_count', _int), | ||
'recent_stats.relay_failures.insufficient_measurements': ('recent_measurements_excluded_few_count', _int), | ||
'recent_stats.relay_failures.stale': ('recent_measurements_excluded_old_count', _int), | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is this the right way to do this? Just include a file from stem? Doesn't this mean we need to keep track of stem's revisions to this file? :/
Also if this is the right way to do this, then perhaps we should mention which revision of this file this is (commit id). Also, why does it say "To be removed on Stem's release 1.8.0"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only until the next release, i think there're stem's releases every ~6 months.
And so far this file has only changed when after adding more headers to
sbws
i create an issue instem
so it gets added too.With this change, instead i could directly create a PR in stem and duplicate it in
sbws
until the next release.Not sure is there's a better way, but i definitely don't want to wait 8 months again to do a sbws release waiting for a stem release and it's not a solution to change the dependency to stem's git master because that does not work for system packages (Debian).
Do you think it would be better i just add the stem's bandwidth_file.py as it is currently?.
I can add the commit id (in v3bwfile if i add the stem's bandwidth_file.py as it's)
Because that's the next Stem's release, i can add this comment somewhere. Where it would fit better?.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if i add the file as it is, i also need to copy the
__init__.py
Thinking more about this, i think it's better to leave only that part, to follow better what i substitute.
I could also do a
try import
and if there's nobandwidth_file.py
in stem then leave the old code + the refactor.But i think duplicating all that code when as i said, the file won't change unless someone open a ticket, and there'll be a new stem release soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See small changes in fixup