Skip to content

Always consume the no_throttle keyword argument.#11126

Merged
balloob merged 1 commit intohome-assistant:devfrom
andreacampi:fix-hue
Dec 14, 2017
Merged

Always consume the no_throttle keyword argument.#11126
balloob merged 1 commit intohome-assistant:devfrom
andreacampi:fix-hue

Conversation

@andreacampi
Copy link
Copy Markdown
Contributor

@andreacampi andreacampi commented Dec 13, 2017

The current code relies on the assumption that the first invocation will never specify no_throttle=True.
However that puts us in a pickle when writing unit tests: if we had a fictitious:

def setup_platform():
update()

@Throttle(MIN_TIME_BETWEEN_SCANS)
def update():
pass

Then given multiple tests, the second and some of subsequent tests would be throttled (depending on timing).
But we also can't change that code to call `update(no_throttle=True)' because that's not currently accepted.

This diff shouldn't change the visibile behavior of any component, but allows this extra flexibility.

Description:

Related issue (if applicable): fixes #11125

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

The current code relies on the assumption that the first invocation will never specify no_throttle=True.
However that puts us in a pickle when writing unit tests: if we had a fictitious:

  def setup_platform():
    update()

  @Throttle(MIN_TIME_BETWEEN_SCANS)
  def update():
    pass

Then given multiple tests, the second and some of subsequent tests would be throttled (depending on timing).
But we also can't change that code to call `update(no_throttle=True)' because that's not currently accepted.

This diff shouldn't change the visibile behavior of any component, but allows this extra flexibility.
@andreacampi
Copy link
Copy Markdown
Contributor Author

@balloob this fixes a bug in my previous diff, would you merge?

@balloob balloob merged commit e627544 into home-assistant:dev Dec 14, 2017
akatrevorjay added a commit to akatrevorjay/home-assistant that referenced this pull request Dec 15, 2017
…into dev

* 'dev' of https://github.com/home-assistant/home-assistant:
  Disable html5 notify dependency (home-assistant#11135)
  ISY994 sensor improvements (home-assistant#10805)
  Allow using more than one keyboard remote (home-assistant#11061)
  set default utc offset to 0 (home-assistant#11114)
  Add problem device class (home-assistant#11130)
  Always consume the no_throttle keyword argument. (home-assistant#11126)
  Skip HASS emulated Hue bridges from detection. (home-assistant#11128)
  update pyripple (home-assistant#11122)
  Add media position properties (home-assistant#10076)
  Fixed typo in automation.py (home-assistant#11116)
@fabaff fabaff mentioned this pull request Dec 16, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Mar 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hue component broken after a recent refactoring

4 participants