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

home-assistant fails to load when component doesn't have dependencies #41028

Closed
peterhoeg opened this issue May 24, 2018 · 7 comments · Fixed by #41080
Closed

home-assistant fails to load when component doesn't have dependencies #41028

peterhoeg opened this issue May 24, 2018 · 7 comments · Fixed by #41080

Comments

@peterhoeg
Copy link
Member

Issue description

Before HA 0.69, the upstream requirements file listed notify.matrix however since 0.69 that is no longer the case. As a consequence when trying to add a matrix notifier, things will fail with:

error: attribute 'notify.matrix' missing, at /home/peter/src/active/nixpkgs_unstable/pkgs/servers/home-assistant/default.nix:69:28

Adding in a dummy like this in component-packages.nix makes things work again:

    "notify.matrix" = ps: with ps; [  ];

Cc: @dotlambda

Technical details

 - system: `"x86_64-linux"`
 - host os: `Linux 4.16.9, NixOS, 18.09.git.3864e6e9515M (Jellyfish)`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.0.2`
 - channels(root): `""`
 - channels(peter): `""`
 - nixpkgs: `/home/peter/src/active/nixpkgs_unstable`
@dotlambda
Copy link
Member

dotlambda commented May 24, 2018

Are you setting extraComponents by hand? The module should only add available ones.
We could avoid this problem by filtering extraComponents to only include the ones that are also in availableComponents. However, we should print a warning that the user has added an unavailable component.
EDIT: "Unavailable" is probably not the best description. There is either no component of that name or it isn't listed in requirements_all.txt.

@peterhoeg
Copy link
Member Author

Sorry, I should have been clearer. There is a notify.matrix component but it is not listed in requirements.txt because the functionality is provided using the matrix component (matrix vs notify.matrix) - see https://www.home-assistant.io/components/matrix/ and https://www.home-assistant.io/components/notify.matrix/

@dotlambda
Copy link
Member

dotlambda commented May 24, 2018

I know, but there was only notify.matrix until recently (and it was listed in requirements_all.txt).

Since the notify.matrix component now requires the matrix component, you should be just fine if you're using autoExtraComponents.

However, using something along the lines of

pkgs.home-assistant.override {
  extraComponents = [ "notify.matrix" ];
}

indeed gives you an error because notify.matrix was removed from requirements_all.txt.
That has a somewhat positive effect of making the user check why it was removed/what it was replaced by.

In your opinion, what would be the ideal way to deal with this case?

@dotlambda
Copy link
Member

I think we have three options (ordered by difficulty of implementation):

  1. Leave it as is and maybe add some bit of documentation on which modules can/should be added to extraComponents.
  2. As described above, ignore modules that are in extraComponents but not in availableComponents, i.e. that don't occur in requirements_all.txt. Maybe print a warning that the module name might be misspelled.
  3. Store a list of all Home Assistant components, not just those in requirements_all.txt, and allow any of these components to be in extraComponents. Abort with an error if a member of extraComponents is not in the list of all components. If it is done this way, some people might be confused as to which components to add to extraComponents (e.g. matrix and not notify.matrix because only the former has dependencies).

@peterhoeg
Copy link
Member Author

How about a mapping of components to other component dependencies so that the parse script can generate the correct entry?

I don't know if there are others than the matrix component but it wouldn't be too far fetched to assume that this was the case.

So notify.matrix gets written out to the components file with the dependencies of matrix.

@dotlambda
Copy link
Member

There are definitely others, with the most notable one being the history component which depends on the recorder component which requires the SQLAlchemy package.
There is currently no way of detecting these dependencies in parse-requirements.py. However, it is possible using the DEPENDENCIES list exported by each component, see e.g. https://github.com/home-assistant/home-assistant/blob/0.69.1/homeassistant/components/notify/matrix.py#L21. This would require downloading the Home Assistant source tarball, importing each component and looking up its dependencies in parse-requirements.py. There is https://github.com/home-assistant/home-assistant/blob/0.69.1/script/gen_requirements_all.py which can serve as a guide on how to do that.

@peterhoeg
Copy link
Member Author

The alternative is to define in parse-requirements.py which components have dependencies on others. That would have to be manually maintained (which isn't ideal) but would be a quick way to get things going.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants