home-assistant: init at 0.62.1 #34188

dotlambda · 2018-01-23T10:39:38Z

Regarding --skip-pip:
When I disable that option, not even the frontend component can be loaded because home-assistant isn't able to import pip. I have no idea why.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
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 nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

/cc @FRidh @f-breidenstein @flokli

dotlambda · 2018-01-23T14:56:29Z

I have modified the derivation to use an older version of yarl, since yarl-1.0.0 removed the quote and unquote functions used by home-assistant: https://github.com/aio-libs/yarl/blob/master/CHANGES.rst#100-2018-01-15.

dotlambda · 2018-01-23T16:08:27Z

@FRidh Could you please have a look at whether I implemented extraPackages exactly as you intended it to be?

FRidh

Looks good. I haven't reviewed the module though.

FRidh · 2018-01-23T16:12:50Z

pkgs/development/python-modules/astral/default.nix

+buildPythonPackage rec {
+  pname = "astral";
+  version = "1.4";
+  name  = "${pname}-${version}";


FRidh · 2018-01-23T16:13:43Z

pkgs/servers/home-assistant/default.nix

+  python = python3.override {
+    packageOverrides = self: super: {
+      yarl = super.yarl.overridePythonAttrs (oldAttrs: rec {
+        name = "${oldAttrs.pname}-${version}";


if yarl is without name, then name can be dropped here as well.

FRidh · 2018-01-23T16:13:53Z

pkgs/servers/home-assistant/default.nix

+        };
+      });
+      aiohttp = super.aiohttp.overridePythonAttrs (oldAttrs: rec {
+        name = "${oldAttrs.pname}-${version}";


aiohttp still had a name attribute. I've removed it now.

FRidh · 2018-01-23T16:14:31Z

pkgs/servers/home-assistant/default.nix

+      });
+    };
+  };
+  hass-frontend = python.pkgs.callPackage ./frontend.nix { };


having this inside packageOverrides and then self.callPackage is I think a bit nicer, but it is not necessary.

It also has the advantage of being able to modify the frontend package.

FRidh · 2018-01-23T16:16:21Z

pkgs/servers/home-assistant/default.nix

+    requests pyyaml pytz pip jinja2 voluptuous typing aiohttp yarl async-timeout chardet astral certifi
+    # From running hass
+    hass-frontend sqlalchemy netdisco xmltodict user-agents
+  ] ++ extraPackages python.pkgs;


I suggest having extraPackages python.pkgs either in the let expression assigned to something, or have parentheses here around it, just for clarity. I don't know what is better. What do you think?

FRidh · 2018-01-23T16:16:44Z

pkgs/servers/home-assistant/default.nix

+
+  makeWrapperArgs = [ "--add-flags --skip-pip" ];
+
+  doCheck = false;


comment with reason?

FRidh · 2018-01-23T16:18:59Z

pkgs/top-level/all-packages.nix

@@ -11954,6 +11954,8 @@ with pkgs;

  hiawatha = callPackage ../servers/http/hiawatha {};

+  home-assistant = python3Packages.callPackage ../servers/home-assistant { };


This should simply be callPackage because we do not have any Python packages to supply as arguments.

That's true, but it will hopefully change in the future when home-assistant allows newer versions of the dependencies :)

If we have packages from python-packages.nix and packages defined locally (read: the referred file) then we need to update the Python package set with packageOverrides. There will not be any use for defining Python packages as parameters, and as such no case for using pythonPackages.callPackage.

I know, but I hope that home-assistant will just work with the standard python package set in the future.

Currently the not overridden Python package set (see arguments python and isPy3k) is mixed with the overridden set. That should not happen.

I know, but I hope that home-assistant will just work with the standard python package set in the future.

Very unlikely. We have a lot of Python applications, and for the larger ones we typically need to provide overrides.

FRidh · 2018-01-23T16:20:42Z

Regarding --skip-pip:
When I disable that option, not even the frontend component can be loaded because home-assistant isn't able to import pip. I have no idea why.

Provide pip conditionally?

Could you please have a look at whether I implemented extraPackages exactly as you intended it to be?

Exactly as intended! 👍

dotlambda · 2018-01-23T16:48:48Z

Providing pip conditionally won't work because home-assistant will still check whether it is running inside a virtualenv. I've previously tried removing https://github.com/home-assistant/home-assistant/blob/master/homeassistant/util/package.py#L10 but some home-assistant component seems to import running_under_virtualenv.

dotlambda · 2018-01-23T19:33:03Z

It would be nice if someone else could also test the NixOS module. The only problem I've noticed so far that the map doesn't show up on http://localhost:8123/map.

flokli · 2018-01-23T20:04:04Z

It might still take me one or two days to give this a spin, but could you provide a nixos test for it, and maybe add a way to configure it using the module directly too, using `toYAML`?

dotlambda · 2018-01-23T21:20:26Z

Do you have an idea of what exactly to test? I haven't yet worked with home-assistant that much.

flokli · 2018-01-23T21:33:44Z

Maybe simply asserting something is listening on the web interface port and replying with a 2xx code?

dotlambda · 2018-01-25T13:16:21Z

@flokli I have added a config option that allows specifying the configuration.yaml as a Nix attribute set. However, I wasn't sure how to deal with the case of an existing configuration.yaml file. I decided to simply delete the old one in this case.
See https://github.com/dotlambda/nixpkgs/blob/home-assistant/nixos/modules/services/misc/home-assistant.nix#L58.

dotlambda · 2018-01-26T00:45:53Z

Finally, a NixOS test has been added. It checks whether

the config option works as expected and
web interface and API are accessible.

dotlambda · 2018-01-26T13:04:57Z

Btw, should I add the test to nixos/release.nix?

dotlambda · 2018-01-26T13:32:46Z

And finally, I think the package should actually go into python-packages.nix since it also provides a Python API: https://dev-docs.home-assistant.io/en/dev/

dotlambda · 2018-01-30T10:07:48Z

Removed from python-packages.nix because this should be a consistent set of packages.
Also updated to 0.62.1.
@FRidh Is there anything left to do?

@GrahamcOfBorg build home-assistant
@GrahamcOfBorg test home-assistant

GrahamcOfBorg

Success on x86_64-linux (full log)

Partial log (click to expand)

tests/components/test_init.py ............                               [ 72%]
tests/components/test_introduction.py .                                  [ 72%]
tests/components/test_logger.py ....                                     [ 75%]
tests/components/test_script.py .....                                    [ 78%]
tests/components/test_shell_command.py ......                            [ 82%]
tests/components/test_system_log.py ..............                       [ 91%]
tests/components/test_websocket_api.py ..............                    [100%]

========================= 159 passed in 10.68 seconds ==========================
/nix/store/h15jk268357qxdif5kfkjklih96641pa-homeassistant-0.62.1

GrahamcOfBorg

Success on x86_64-linux (full log)

Partial log (click to expand)

hass: exit status 1
syncing
hass: running command: sync
hass: exit status 0
test script finished in 238.19s
cleaning up
killing hass (pid 596)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/tmp/nix-build-vm-test-run-home-assistant.drv-0/vde1.ctl': Directory not empty
/nix/store/2crgxdwq1adi8hl09g52hl30v65gxv34-vm-test-run-home-assistant

GrahamcOfBorg

Success on aarch64-linux (full log)

Partial log (click to expand)

tests/components/test_init.py ............                               [ 72%]
tests/components/test_introduction.py .                                  [ 72%]
tests/components/test_logger.py ....                                     [ 75%]
tests/components/test_script.py .....                                    [ 78%]
tests/components/test_shell_command.py ......                            [ 82%]
tests/components/test_system_log.py ..............                       [ 91%]
tests/components/test_websocket_api.py ..............                    [100%]

========================= 159 passed in 31.29 seconds ==========================
/nix/store/64s228galv96j14dl4xbnmagqbydizvz-homeassistant-0.62.1

FRidh

The Python expressions look good to me. Just the small callPackage change is needed. I have not reviewed the module.

FRidh · 2018-01-30T10:19:17Z

pkgs/top-level/all-packages.nix

@@ -11954,6 +11954,8 @@ with pkgs;

  hiawatha = callPackage ../servers/http/hiawatha {};

+  home-assistant = python3Packages.callPackage ../servers/home-assistant { };


Currently the not overridden Python package set (see arguments python and isPy3k) is mixed with the overridden set. That should not happen.

I know, but I hope that home-assistant will just work with the standard python package set in the future.

Very unlikely. We have a lot of Python applications, and for the larger ones we typically need to provide overrides.

dotlambda · 2018-01-30T10:25:44Z

Now I don't use python3Packages.callPackage anymore.

dotlambda · 2018-02-01T08:42:06Z

@GrahamcOfBorg build home-assistant
@GrahamcOfBorg test home-assistant

@FRidh Please merge :)
I will then create another PR with a suggestion for computing the necessary component dependencies from the config in the next days.

GrahamcOfBorg · 2018-02-01T08:42:15Z

Success on x86_64-linux (full log)

Partial log (click to expand)

/nix/store/h15jk268357qxdif5kfkjklih96641pa-homeassistant-0.62.1

FRidh · 2018-02-01T08:44:53Z

Thanks!

GrahamcOfBorg · 2018-02-01T08:45:03Z

Success on x86_64-linux (full log)

Partial log (click to expand)

hass: exit status 1
syncing
hass: running command: sync
hass: exit status 0
test script finished in 47.32s
cleaning up
killing hass (pid 593)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/tmp/nix-build-vm-test-run-home-assistant.drv-0/vde1.ctl': Directory not empty
/nix/store/h8v8n6kaj88p98fxg5xdfxk7aqxng93l-vm-test-run-home-assistant

GrahamcOfBorg · 2018-02-01T08:46:22Z

Success on aarch64-linux (full log)

Partial log (click to expand)

tests/components/test_init.py ............                               [ 72%]
tests/components/test_introduction.py .                                  [ 72%]
tests/components/test_logger.py ....                                     [ 75%]
tests/components/test_script.py .....                                    [ 78%]
tests/components/test_shell_command.py ......                            [ 82%]
tests/components/test_system_log.py ..............                       [ 91%]
tests/components/test_websocket_api.py ..............                    [100%]

========================= 159 passed in 31.11 seconds ==========================
/nix/store/64s228galv96j14dl4xbnmagqbydizvz-homeassistant-0.62.1

GrahamcOfBorg · 2018-02-01T08:47:37Z

Success on aarch64-linux (full log)

Partial log (click to expand)

hass: exit status 1
syncing
hass: running command: sync
hass: exit status 0
test script finished in 58.74s
cleaning up
killing hass (pid 627)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/b3hy82vi2xn51jsrp2dd1yj6q9rbnl3g-vm-test-run-home-assistant

yesbox · 2018-02-01T20:19:06Z

pkgs/servers/home-assistant/default.nix

+  pname = "homeassistant";
+  version = "0.62.1";
+
+  diabled = !isPy3k;


Forgot to remove this?

You're right. I'll remove it with the next update.

dotlambda requested a review from FRidh as a code owner January 23, 2018 10:39

GrahamcOfBorg added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jan 23, 2018

dotlambda force-pushed the home-assistant branch 2 times, most recently from f1be9c4 to f02b748 Compare January 23, 2018 14:54

dotlambda force-pushed the home-assistant branch from f02b748 to de0f855 Compare January 23, 2018 15:58

dotlambda force-pushed the home-assistant branch from de0f855 to 353b3dd Compare January 23, 2018 16:19

FRidh requested changes Jan 23, 2018

View reviewed changes

FRidh added the 6.topic: python label Jan 23, 2018

FRidh mentioned this pull request Jan 23, 2018

[WIP] Home Assistant #33675

Closed

12 tasks

dotlambda force-pushed the home-assistant branch 3 times, most recently from 021e921 to 78baeca Compare January 23, 2018 16:45

dotlambda force-pushed the home-assistant branch 2 times, most recently from 0ac8c82 to 7471f2e Compare January 23, 2018 17:22

dotlambda force-pushed the home-assistant branch from 7471f2e to a92fddf Compare January 25, 2018 13:09

dotlambda force-pushed the home-assistant branch from a92fddf to bcfbad8 Compare January 26, 2018 00:42

dotlambda force-pushed the home-assistant branch from bcfbad8 to 98b6e17 Compare January 26, 2018 17:10

flokli and others added 4 commits January 30, 2018 11:01

astral: init at 1.4

ab02f54

pythonPackages.pytest-aiohttp: init at 0.3.0

9d39268

pythonPackages.aiohttp: remove name attribute

c68a165

Add f-breidenstein as maintainer

6a2ead5

dotlambda force-pushed the home-assistant branch from 6b0396c to 96445cc Compare January 30, 2018 10:05

dotlambda changed the title ~~home-assistant: init at 0.62.0~~ home-assistant: init at 0.62.1 Jan 30, 2018

GrahamcOfBorg reviewed Jan 30, 2018

View reviewed changes

FRidh reviewed Jan 30, 2018

View reviewed changes

home-assistant: init at 0.62.1

5bf6d94

dotlambda force-pushed the home-assistant branch from 96445cc to cfc242b Compare January 30, 2018 10:24

dotlambda force-pushed the home-assistant branch 2 times, most recently from 04e3220 to 6e6f5c6 Compare January 31, 2018 03:59

Robert Schütz added 2 commits January 31, 2018 12:30

home-assistant: add NixOS module

bacbc48

home-assistant: add NixOS test

0604c07

dotlambda force-pushed the home-assistant branch from 6e6f5c6 to 0604c07 Compare January 31, 2018 11:35

FRidh merged commit d30735f into NixOS:master Feb 1, 2018

dotlambda deleted the home-assistant branch February 1, 2018 08:49

yesbox reviewed Feb 1, 2018

View reviewed changes

dotlambda mentioned this pull request Feb 2, 2018

Yarl removed unquote home-assistant/core#11906

Closed

dotlambda mentioned this pull request May 4, 2024

Request for a Zulip account for governance discussion NixOS/foundation#143

Closed


		makeWrapperArgs = [ "--add-flags --skip-pip" ];

		doCheck = false;

		@@ -11954,6 +11954,8 @@ with pkgs;

		hiawatha = callPackage ../servers/http/hiawatha {};

		home-assistant = python3Packages.callPackage ../servers/home-assistant { };

home-assistant: init at 0.62.1 #34188

home-assistant: init at 0.62.1 #34188

Conversation

dotlambda commented Jan 23, 2018

Things done

dotlambda commented Jan 23, 2018

dotlambda commented Jan 23, 2018

FRidh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FRidh commented Jan 23, 2018

dotlambda commented Jan 23, 2018

dotlambda commented Jan 23, 2018

flokli commented Jan 23, 2018 via email

dotlambda commented Jan 23, 2018

flokli commented Jan 23, 2018 via email

dotlambda commented Jan 25, 2018 • edited Loading

dotlambda commented Jan 26, 2018

dotlambda commented Jan 26, 2018

dotlambda commented Jan 26, 2018

dotlambda commented Jan 30, 2018

GrahamcOfBorg left a comment

Choose a reason for hiding this comment

GrahamcOfBorg left a comment

Choose a reason for hiding this comment

GrahamcOfBorg left a comment

Choose a reason for hiding this comment

FRidh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotlambda commented Jan 30, 2018

dotlambda commented Feb 1, 2018

GrahamcOfBorg commented Feb 1, 2018

FRidh commented Feb 1, 2018

GrahamcOfBorg commented Feb 1, 2018

GrahamcOfBorg commented Feb 1, 2018

GrahamcOfBorg commented Feb 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotlambda commented Jan 25, 2018 •

edited

Loading