project8 / dripline-python

python implementation of project8/dripline
Other
2 stars 0 forks source link

refactor scheduler #18

Closed wcpettus closed 7 years ago

wcpettus commented 7 years ago

Retain only schedule-specific items, logger-specific pieces dumped into spime, provider updated to reflect new split. Verified to work in the T2 zone.

It still doesn't neatly handle the pinger startup, but it does most of the other things we want.

It will require a coordinated release of dragonfly (to update the pinger code) and hardware (to update every setup_call naming); these are staged. I considered rerouting on_set for logging_status of provider to schedule_status, but thought that would just prolong the pain.

wcpettus commented 7 years ago

Migrating some conversation out of Slack:

  1. Backwards compatibility can be ensured for spimes by adding to spime.py

    @property
    def log_interval(self):
    return self.schedule_interval
    @log_interval.setter
    def log_interval(self, value):
    self.schedule_interval = value

    (and something similar for for logging_status)

  2. Backwards compatibility of setup_calls can be preserved by adding a redundant method to provider

    @property
    def logging_status(self):
        logger.warning('use of logging_status is deprecated, switch to using schedule_status')
        return self.schedule_status
    @logging_status.setter
    def logging_status(self, value):
        logger.warning('use of logging_status is deprecated, switch to using schedule_status for setup_call')
        self.schedule_status = value
wcpettus commented 7 years ago

Additional thought - any endpoint which is not logged but is below a provider which has the setup_call will spam a warning that the "schedule loop interval must be > 0". This could be bypassed by making the provider schedule_status.setter have a recognized value that will cause it to skip an endpoint (default is 0, so this requires a deliberate setting)

    if endpoint.schedule_interval == -1:
        logger.debug('skipping {} because schedule_interval set to -1'.format(endpoint.name))
        continue
laroque commented 7 years ago

Those new changes look good. Are there more tests before we put the merge through or is it ready?

wcpettus commented 7 years ago

It's running stably on dorian for the last ~20 hours. So both pinger and spimes are confirmed to be working. I think that counts as merge-ready.

wcpettus commented 7 years ago

fixed schedule_interval.setter to restart loop for new interval to take effect