1. Team SimPy
  2. simpy
  3. simpy

Pull requests

#38 Declined
Repository
simpy-timed-failure
Branch
default
Repository
simpy
Branch
default

Implement TimedFailure

Author
  1. luensdorf
Reviewers
Description

RFC: Add an event type for delayed failures.

The name TimedFailure sucks. I did not modify Timeout because the isinstance() check is a performance hit.

Instead of finding a better name we could also streamline the API by adding a Success class (which would be almost identical to Timeout) and a Failure class. Both would accept an optional delay parameter. For example:

env.success('foo') # delay defaults to zero
env.success('foo', delay=2)
env.success('foo', 2)

env.failure(ValueError('spam')) # also delay of zero per default
env.failure(ValueError('spam'), delay=2)
  • Learn about pull requests

Comments (12)

  1. Andreas Beham

    What about TimeBomb ;-)

    Sounds confusing, seems like you want to succeed or fail the whole environment. As mentioned in issue #54 if we would stop scheduling timeouts during init we can create timeouts normally using any delay and succeed or fail them manually.

    1. luensdorf author

      Yeah, it sounds a bit confusing. If you don't use the convenience methods, it becomes a bit clearer:

      def p(env):
           yield Success(env, 'foo', delay=2)
      

      Finding names is always painful :)

  2. Stefan Scherfke

    luensdorf One of your favorite words: “feature creep”

    Why not just let the user do:

    def timebomb(env, delay, exc):
        yield env.timeout(delay)
        raise exc
    
    def my_proc(env):
        yield timebomb(env, 3, OnoesError())
    
    1. luensdorf author

      This is too slow because an unneccessary process is created.

      This isn't feature creep but improves consistency. Events can succeed and fail. Timeouts can't and this is inconsistent.

      1. Andreas Beham

        Alternative:

        def __init__(self, env, delay, value=None, ok=True):
          self.env = env
          self.callbacks = []
          self._delay = delay
          self._value = value
          self.ok = ok
          env.schedule(self, URGENT)
        
        env.timeout(2, value=OnoesError(), ok=False)
        

        Requires that you always use an error if you combine with ok=False.

        1. Andreas Beham

          Still I'm beginning to think that calling env.schedule in init is actually an unwanted side-effect. I think it would be better to not have anything schedule anything if you just create it, but only if you create it through the environment shortcuts. That would also give a little distinction between x = Timeout(env, 2) and env.timeout(2).

          Thinking further in that direction, you could remove succeed, fail and trigger from all events and offer them as methods of the environment, e.g.: env.succeed(event). That would also solve the visibility of schedule, because nobody needs access to it anymore.

          1. luensdorf author

            I don't understand how this is supposed to work. Could you explain, how the user is supposed to schedule a timeout in say 2 timeunits? But lets move that discussion to the issue you created.

        1. luensdorf author

          Please don't say our coding conventions, you are the only one enforcing them. I've always said that I've objections against some flake8 warnings and explained why I think so. You could have easily disabled on the offending whitespace warnings as a compromise or at least try to explain the benefits.

          Anyhow, I will now stop spending time discussing non-technical flake8 whitespace stuff and let you have it your way.