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

home-assistant: upgrade to 0.104.1, add support for zwave/zha adapters, add various python modules for components #77883

Closed
wants to merge 9 commits into from
Closed

home-assistant: upgrade to 0.104.1, add support for zwave/zha adapters, add various python modules for components #77883

wants to merge 9 commits into from

Conversation

colemickens
Copy link
Member

Motivation for this change
  • Upgrade home-assistant to latest (0.104.1)
  • Add the home-assistant service user to the dialout group so that it can access certain zwave/zha adapters
  • Add the following modules:
    • denonavr: (denon component)
    • braviarc-homeassistant: (bravia component)
    • plexauth, plexapi, plexwebsocket: (plex component)
    • flux_led, getmac: (magichome/fluxled component)

cc: @Mic92

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.

@colemickens
Copy link
Member Author

(the new commits were just me fixing up the maintainer for the new python packages to be myself.)

@ofborg ofborg bot requested review from dotlambda and globin January 17, 2020 08:32
jonringer
jonringer reviewed Jan 17, 2020
View reviewed changes
pkgs/development/python-modules/flux-led/default.nix Outdated
Comment on lines 9 to 11
nativeBuildInputs = [ ];
buildInputs = [ ];

Copy link
Contributor

Choose a reason for hiding this comment

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

remove if not needed please

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

jonringer
jonringer reviewed Jan 19, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

I would really like to see some unit tests. Makes maintaining the python package set easier, especially during large package bumps.

@colemickens
Copy link
Member Author

This is my first time adding a python package, I think. I assume you're meaning to enable the unit tests for these packages themselves? I will look into this, this makes sense. I was just reviewing some other PRs and saw a comment that pypi doesn't include tests and I was wondering if that meant I was not getting tests as well.

If this isn't a correct understanding, please let me know.

@jonringer
Copy link
Contributor

didn't realise it was your first time dealing with python. A lot of times, packages wont package tests even when making source distributions on PyPi, so we have to check out from github or gitlab so we can build from source + test to ensure the some assumptions weren't broken outside of normal package usage.

jonringer
jonringer reviewed Jan 19, 2020
View reviewed changes
pkgs/servers/home-assistant/default.nix
@@ -85,7 +82,7 @@ in with py.pkgs; buildPythonApplication rec {
owner = "home-assistant";
repo = "home-assistant";
rev = version;
sha256 = "1492q4icyhvz30fw5ysrwlnsls4iy5pv62ay3vq1ygcfnlapkqhl";
sha256 = "139pfcnnhi9hr6y3fyd2qaf7cmiili5alqwm4i9gpx6lwxibvyiz";
Copy link
Contributor

Choose a reason for hiding this comment

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

hash mismatch in fixed-output derivation '/nix/store/q38aa7n70ci5sy5c6nv6fvszs5ss2mqj-source':
  wanted: sha256:139pfcnnhi9hr6y3fyd2qaf7cmiili5alqwm4i9gpx6lwxibvyiz
  got:    sha256:0hgf8sc6isaaxx4jbm65pl16v4qnw4wff9rw5rzd2a4dza0qr2lr
cannot build derivation '/nix/store/x33a930rbfins9dkazzh17jvi0q9m610-homeassistant-0.104.1.drv': 1 dependencies couldn't be built
error: build of '/nix/store/x33a930rbfins9dkazzh17jvi0q9m610-homeassistant-0.104.1.drv' failed
Suggested change
sha256 = "139pfcnnhi9hr6y3fyd2qaf7cmiili5alqwm4i9gpx6lwxibvyiz";
sha256 = "0hgf8sc6isaaxx4jbm65pl16v4qnw4wff9rw5rzd2a4dza0qr2lr";

@colemickens
Copy link
Member Author

Thanks for the attention so far, @jonringer. There are enough changes and things worthy of review that I'm going to split this PR. It was greedy optimistic of me to try to do all of this at once.

Replacing this one:

@colemickens
Copy link
Member Author

Per my last reply just now, I have updated the sources for the python packages per your comments here, but I need to go back and verify that was sufficient to trigger testing, or if I need to add an attribute...

@jonringer
Copy link
Contributor

sounds good, I responded to most of them

@colemickens colemickens deleted the nixpkgs-homeass branch January 30, 2020 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer left review comments

@FRidh FRidh Awaiting requested review from FRidh

@dotlambda dotlambda Awaiting requested review from dotlambda

@globin globin Awaiting requested review from globin

Assignees
No one assigned
Labels
6.topic: nixos 6.topic: python 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@colemickens @jonringer