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

Avoid calling yr update every second for a minute ones every hour #16731

Merged
merged 2 commits into from
Sep 20, 2018

Conversation

Danielhiversen
Copy link
Member

@Danielhiversen Danielhiversen commented Sep 20, 2018

Description:

Avoid calling yr update every second for a minute ones every hour

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

@ghost ghost assigned Danielhiversen Sep 20, 2018
@ghost ghost added the in progress label Sep 20, 2018
@@ -91,7 +91,7 @@
async_add_entities(dev)

weather = YrData(hass, coordinates, forecast, dev)
async_track_utc_time_change(hass, weather.updating_devices, minute=31)
async_track_utc_time_change(hass, weather.updating_devices, minute=31, second=0)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (84 > 79 characters)

@Danielhiversen Danielhiversen changed the title Avoid calling yr update every second for a minute one every hour Avoid calling yr update every second for a minute ones every hour Sep 20, 2018
@balloob balloob added this to the 0.78.1 milestone Sep 20, 2018
@balloob
Copy link
Member

balloob commented Sep 20, 2018

omg.

@balloob
Copy link
Member

balloob commented Sep 20, 2018

I wonder if we should change the defaults on this function. If you pass in minute, seconds defaults to 0.

@Danielhiversen
Copy link
Member Author

Danielhiversen commented Sep 20, 2018

It is not fetching new data from the server that often, but it is iterating over the self.data dict.

Yeah, it might be a good idea to change the default.
I have done the same mistake for the time trigger in automations.

@balloob
Copy link
Member

balloob commented Sep 20, 2018

Oh, it's throttled, good. Great catch

@balloob balloob merged commit 93af3c5 into dev Sep 20, 2018
@balloob balloob deleted the yr_update branch September 20, 2018 09:31
@ghost ghost removed the in progress label Sep 20, 2018
@balloob balloob removed this from the 0.78.1 milestone Sep 20, 2018
@balloob
Copy link
Member

balloob commented Sep 20, 2018

Removing it from milestone as it's not critical.

@balloob balloob mentioned this pull request Sep 28, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed integration: yr small-pr PRs with less than 30 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants