Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add is_reasonably_live property for NetworkStatusDocument #86

Open
jbrown299 opened this issue Jan 17, 2021 · 1 comment
Open

Add is_reasonably_live property for NetworkStatusDocument #86

jbrown299 opened this issue Jan 17, 2021 · 1 comment

Comments

@jbrown299
Copy link

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
Copy link
Contributor

atagar commented Jan 17, 2021

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants