torproject / stem

Python controller library for Tor
https://stem.torproject.org/
GNU Lesser General Public License v3.0
279 stars 76 forks source link

Add `is_reasonably_live` property for `NetworkStatusDocument` #86

Open jbrown299 opened 3 years ago

jbrown299 commented 3 years ago

Inside NetworkStatusDocumentV3 you have is_fresh method:

  def is_fresh(self):
    """
    Checks if the current time is between this document's **valid_after** and
    **fresh_until** timestamps. To be fresh means this should be the latest
    consensus.

    .. versionadded:: 1.8.0

    :returns: **True** if this consensus is presently fresh and **False**
      otherwise
    """

    return self.valid_after < datetime.datetime.utcnow() < self.fresh_until
  1. May be better to define it as property.
  2. May be better follow original tor source client which naming it as networkstatus_is_live.
  3. We need second propery is_reasonably_live like networkstatus_consensus_reasonably_live. It is used when tor client try use old consensus but not so old.
  4. Need to double check is it really < and > or <= and >=

From torpy sources:


    @property
    def is_live(self):
        # tor ref: networkstatus_is_live
        return self.valid_after <= datetime.utcnow() <= self.valid_until

    @property
    def is_reasonably_live(self):
        # tor ref: networkstatus_consensus_reasonably_live
        return self.valid_after - timedelta(hours=24) <= datetime.utcnow() <= self.valid_until + timedelta(hours=24)
atagar commented 3 years ago

May be better to define it as property.

Hi James. This is an interesting idea but I'm torn. The rule of thumb I use is 'property for static values, method for dynamic'. Personally I'd find it confusing to have our object's is_live attribute be True now, and False five minutes from now.

Methods by contrast are clearly evaluated at runtime, fulfilling the python axium Explicit is better than implicit..

Thoughts?