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.

Sign up for GitHub

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

Jump to bottom

homeassistant: 0.111.4 -> 0.112.3 #91958

Merged
merged 9 commits into from
Jul 7, 2020
Merged

homeassistant: 0.111.4 -> 0.112.3 #91958

merged 9 commits into from
Jul 7, 2020

Conversation

mweinelt
Copy link
Member

@mweinelt mweinelt commented Jul 1, 2020

Using the glorious ./update.sh tooling. Thanks @Mic92!

Motivation for this change

New upstream release.
https://www.home-assistant.io/blog/2020/07/01/release-112/

Logbook and History are now fast, expect db migrations on startup.

The tests needed a fixup, because the embedded MQTT broker was removed.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Mic92 @dotlambda

@mweinelt
Copy link
Member Author

mweinelt commented Jul 1, 2020

Upgrade on an RPi4 with PostgreSQL and 1.4GB database size on an SSD connected over USB3 took roughly 10 seconds.

2020-07-01 19:42:57 WARNING (Recorder) [homeassistant.components.recorder.migration] Database is about to upgrade. Schema version: 7
2020-07-01 19:42:57 WARNING (Recorder) [homeassistant.components.recorder.migration] Adding columns context_parent_id to table events. Note: this can take several minutes on large databases and slow computers. Please be patient!
2020-07-01 19:42:57 WARNING (Recorder) [homeassistant.components.recorder.migration] Adding columns old_state_id to table states. Note: this can take several minutes on large databases and slow computers. Please be patient!
2020-07-01 19:42:57 WARNING (Recorder) [homeassistant.components.recorder.migration] Adding index `ix_events_context_parent_id` to database. Note: this can take several minutes on large databases and slow computers. Please be patient!
2020-07-01 19:43:02 WARNING (Recorder) [homeassistant.components.recorder.migration] Adding index `ix_events_event_type_time_fired` to database. Note: this can take several minutes on large databases and slow computers. Please be patient!
2020-07-01 19:43:06 WARNING (MainThread) [homeassistant.setup] Setup of recorder is taking over 10 seconds.
2020-07-01 19:43:07 WARNING (Recorder) [homeassistant.components.recorder] Ended unfinished session (id=201 from 2020-06-30 05:30:37.667604+00:00)

The load times of logbook/history are down from ~20s to under less than 3s.

@mweinelt mweinelt requested review from Mic92, colemickens and dotlambda and removed request for Mic92 and colemickens July 1, 2020 18:06
@colemickens
Copy link
Member

colemickens commented Jul 1, 2020 •
edited
Loading

This broke denonavr integration for me. The log has this, which is surprising since defusedxml is already in nixpkgs, so I assumed update.sh would've pulled it in. I only just started looking though (and I don't necessarily expect my issue to block this PR, just wanted to give initial feedback.)

error: https://gist.github.com/colemickens/e814a6e78d29dc487336c242c9b59418

@colemickens
Copy link
Member

Well, as far as I can tell, denonavr->ssdp->defusedxml, so it looks like a possible bug in the update script and it's dependency resolution.

I'm not sure it matters to me anymore though. This release removes denonavr's ability to be configured via YAML. I am looking for a more static home-assistant replacement moving forward. (Something nix-powered would be neat, and could pair nicely with esphome's declarative nature.)

@Mic92
Copy link
Member

Mic92 commented Jul 1, 2020

Well, as far as I can tell, denonavr->ssdp->defusedxml, so it looks like a possible bug in the update script and it's dependency resolution.

I'm not sure it matters to me anymore though. This release removes denonavr's ability to be configured via YAML. I am looking for a more static home-assistant replacement moving forward. (Something nix-powered would be neat, and could pair nicely with esphome's declarative nature.)

mach-nix could provide more precise pinning.

@Mic92
Copy link
Member

Mic92 commented Jul 1, 2020

Well, as far as I can tell, denonavr->ssdp->defusedxml, so it looks like a possible bug in the update script and it's dependency resolution.

I'm not sure it matters to me anymore though. This release removes denonavr's ability to be configured via YAML. I am looking for a more static home-assistant replacement moving forward. (Something nix-powered would be neat, and could pair nicely with esphome's declarative nature.)

Our latest script does resolve transitive dependencies. It was not used here for updating though.

@ofborg ofborg bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Jul 2, 2020
@ofborg ofborg bot requested review from globin and Mic92 July 2, 2020 02:20
@mweinelt
Copy link
Member Author

mweinelt commented Jul 2, 2020

This broke denonavr integration for me. The log has this, which is surprising since defusedxml is already in nixpkgs, so I assumed update.sh would've pulled it in. I only just started looking though (and I don't necessarily expect my issue to block this PR, just wanted to give initial feedback.)

error: https://gist.github.com/colemickens/e814a6e78d29dc487336c242c9b59418

  • denonavr
    • requires "denonavr==0.9.3" "getmac==0.8.2"

Does not depend on any other component, so the update script won't find defusedxml, which is a requirement in the ssdp, ohmconnect, namecheapdns and ihc components.

The denonavr manifest declares some ssdp values, but does not depend on ssdp. This could in fact be an upstream bug.

https://github.com/home-assistant/core/blob/dev/homeassistant/components/denonavr/manifest.json

@mweinelt
Copy link
Member Author

mweinelt commented Jul 2, 2020

I guess they expect ssdp to be loaded through default_config, which depends on it. Did you configure default_config? @colemickens

@Mic92
Copy link
Member

Mic92 commented Jul 4, 2020

There were two patch versions released in the meantime.

@mweinelt
Copy link
Member Author

mweinelt commented Jul 5, 2020

The bump has been pushed.

@colemickens
Copy link
Member

Please don't consider me blocking. I was able to get 0.112.1 working after activating ssdp, so everything's good on my end. I'll pull 0.112.2 and test later, but I don't anticipate problems.

Thanks for the PR and the help!

@mweinelt mweinelt requested review from FRidh and jonringer as code owners July 6, 2020 22:36
@Mic92 Mic92 changed the title homeassistant: 0.111.4 -> 0.112.0 homeassistant: 0.111.4 -> 0.112.2 Jul 6, 2020
@ofborg ofborg bot added 6.topic: python 8.has: package (new) This PR adds a new package labels Jul 6, 2020
@ofborg ofborg bot requested a review from thanegill July 6, 2020 22:44
@ofborg ofborg bot requested a review from kamadorueda July 6, 2020 23:07
@mweinelt mweinelt changed the title homeassistant: 0.111.4 -> 0.112.2 homeassistant: 0.111.4 -> 0.112.3 Jul 6, 2020
@mweinelt
Copy link
Member Author

mweinelt commented Jul 7, 2020

Result of nixpkgs-review pr 91958 1

9 packages built:
- home-assistant
- python37Packages.graphql-core
- python37Packages.graphql-server-core
- python37Packages.influxdb-client
- python37Packages.rx
- python38Packages.graphql-core
- python38Packages.graphql-server-core
- python38Packages.influxdb-client
- python38Packages.rx

1 similar comment
@Mic92
Copy link
Member

Mic92 commented Jul 7, 2020

Result of nixpkgs-review pr 91958 1

9 packages built:
- home-assistant
- python37Packages.graphql-core
- python37Packages.graphql-server-core
- python37Packages.influxdb-client
- python37Packages.rx
- python38Packages.graphql-core
- python38Packages.graphql-server-core
- python38Packages.influxdb-client
- python38Packages.rx

@Mic92 Mic92 merged commit 8a6825f into NixOS:master Jul 7, 2020
@mweinelt mweinelt deleted the hass branch July 7, 2020 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda Awaiting requested review from dotlambda

@globin globin Awaiting requested review from globin

@Mic92 Mic92 Awaiting requested review from Mic92

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

@thanegill thanegill Awaiting requested review from thanegill

@kamadorueda kamadorueda Awaiting requested review from kamadorueda

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python 8.has: clean-up 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mweinelt @colemickens @Mic92