Opened 5 years ago

Closed 5 years ago

#6417 closed defect (implemented)

Controller wrapper method for USEFEATURE messages

Reported by: neena Owned by: neena
Priority: Medium Milestone:
Component: Core Tor/Stem Version:
Severity: Keywords:
Cc: atagar Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

My attempt at this here (Also contains two other documentation/code fixes).

Child Tickets

Change History (5)

comment:1 Changed 5 years ago by neena

Owner: changed from atagar to neena
Status: newassigned

comment:2 Changed 5 years ago by neena

Status: assignedneeds_review

comment:3 Changed 5 years ago by atagar

Status: needs_reviewneeds_revision

Looks great.

+ The following features are currently accepted:
+ * EXTENDED_EVENTS - Requests the extended event syntax. Has the same
+ effect as calling SETEVENTS with EXTENDED. Introduced in
+ 0.1.2.3-alpha, always-on since Tor 0.2.2.1-alpha
+ * VERBOSE_NAMES - Replaces ServerID with LongName in events and GETINFO
+ results. LongName provides a Fingerprint for all routers, an indication
+ of Named status, and a Nickname if one is known. LongName is strictly
+ more informative than ServerID, which only provides either a Fingerprint
+ or a Nickname. Introduced in 0.1.2.2-alpha, always-on since Tor
+ 0.2.2.1-alpha.

Lets not copy the control-spec. I agree that it's nice to have this in our documentation, but if we do this then we get into the business of mirroring this section (or getting stale). Maybe just list the options with a short summary and mention that users can read the control-spec for details?

Usually I'd ask for these requirements to be added to version.py (and spotted that you did for FEATURE_VERBOSENAMES), but 0.1.2.2 and 0.1.2.3 are so incredibly ancient that my humble opinion is that this note should be removed from the control-spec instead. The 0.2.2.1 always-on part might be interesting though. Is there a method to query if a given feature is enabled? If not then we could use this to add one...

def is_feature_enabled(self, feature):
  defaulted_version = None

  if feature == "EXTENDED_EVENTS":
    defaulted_version = stem.version.Requirement.EXTENDED_EVENTS_DEFAULTED
  elif feature == "VERBOSE_NAMES":
    defaulted_version = stem.version.Requirement.VERBOSE_NAMES_DEFAULTED

  if defaulted_version and default_version.meets_requirements(self.get_version()):
    return True
  else:
    return feature in self.enabled_features

Thoughts?

+ if test.runner.require_version(self, stem.version.Requirement.FEATURE_VERBOSENAMES):
+ controller.enable_feature("VERBOSE_NAMES")

Can we do something that requires VERBOSE_NAMES so we can confirm that it is now on?

comment:4 in reply to:  3 Changed 5 years ago by neena

Status: needs_revisionneeds_review

Replying to atagar:

Lets not copy the control-spec. I agree that it's nice to have this in our documentation, but if we do this then we get into the business of mirroring this section (or getting stale). Maybe just list the options with a short summary and mention that users can read the control-spec for details?

Fixed. Didn't mention the control-spec, I think this should suffice.

Usually I'd ask for these requirements to be added to version.py (and spotted that you did for FEATURE_VERBOSENAMES), but 0.1.2.2 and 0.1.2.3 are so incredibly ancient that my humble opinion is that this note should be removed from the control-spec instead. The 0.2.2.1 always-on part might be interesting though.

Removed FEATURE_VERBOSENAMES and added EXTENDED_EVENTS_DEFAULTED and VERBOSE_NAMES_DEFAULTED to stem.version.Requirement. (Though, I left a check for 0.1.2.2 in the test)

def is_feature_enabled(self, feature):
  defaulted_version = None

  if feature == "EXTENDED_EVENTS":
    defaulted_version = stem.version.Requirement.EXTENDED_EVENTS_DEFAULTED
  elif feature == "VERBOSE_NAMES":
    defaulted_version = stem.version.Requirement.VERBOSE_NAMES_DEFAULTED

  if defaulted_version and default_version.meets_requirements(self.get_version()):
    return True
  else:
    return feature in self.enabled_features

Added, with a few modifications

+ if test.runner.require_version(self, stem.version.Requirement.FEATURE_VERBOSENAMES):
+ controller.enable_feature("VERBOSE_NAMES")

Can we do something that requires VERBOSE_NAMES so we can confirm that it is now on?

Yup. Done.

comment:5 Changed 5 years ago by atagar

Resolution: implemented
Status: needs_reviewclosed

Pushed with some minor revisions. Thanks!
https://gitweb.torproject.org/stem.git/commitdiff/6d1f362

Note: See TracTickets for help on using tickets.