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: pythonPackages.zigpy-znp: init at 0.2.2; pythonPackages.zigpy: 0.22.2 -> 0.26.0; pythonPackages.zigpy-cc: 0.5.1 -> 0.5.2; pythonPackages.zigpy-xbee: 0.12.1 -> 0.13.0; pythonPackages.zigpy-zigate: 0.6.1 -> 0.6.2 #101557
Conversation
Result of 16 package built:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed a few components were loosing support during the last upgrade and didn't investigate. I'm sorry that broke your setup, I'll be more careful from now on.
You seem to introduce one spurious change in zigpy-cc
. Apart from that things LGTM.
Result of nixpkgs-review pr 101557
1
16 packages built:
- python37Packages.bellows
- python37Packages.zha-quirks
- python37Packages.zigpy
- python37Packages.zigpy-cc
- python37Packages.zigpy-deconz
- python37Packages.zigpy-xbee
- python37Packages.zigpy-zigate
- python37Packages.zigpy-znp
- python38Packages.bellows
- python38Packages.zha-quirks
- python38Packages.zigpy
- python38Packages.zigpy-cc
- python38Packages.zigpy-deconz
- python38Packages.zigpy-xbee
- python38Packages.zigpy-zigate
- python38Packages.zigpy-znp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kinda expected tests to be present in all those packages and didn't verify that before, since checkInputs
were given and the build passed. But all of these packages show
----------------------------------------------------------------------
Ran 0 tests in 0.000s
and they also don't have a pythonImportsCheck
set up, which is so helpful, since python code can break so easily at runtime. So my recommendation is:
- Try to get tests working
- If they are not packaged in PyPi fetch from Git instead
- If there are no tests or they don't work at all, set
doCheck = false;
- Set up
pythonImportsCheck
instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested these changes on my Home assistant server with my zigbee card. With these changes my Zigbee works again! I've kinda... ignored it failing for a bit.
But it's still worth to look into the tests mentioned by @mweinelt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, the tests are all looking to be working! I have two remaining requests:
Using pytestCheckHook
is advantageous if you are using pytest
, since it allows you to declaratively manage the checkPhase
with disabledTests
and pytestFlagsArray
in the future. You also don't have to specify the checkPhase
that way.
Please replace pytest
with pytestCheckHook
everywhere and drop all checkPhase
s that just call pytest
.
Regarding the commit messages: These packages are python3 only, so their attribute prefix should rather be python3Packages
.
❯ nix-build -A pythonPackages.zigpy
error: asynctest-0.13.0 not supported for interpreter python2.7
It appears the tests are broken for python 3.7.8, possibly due to Martiusweb/asynctest#152. Should I set doCheck false for python 3.7.8? |
On Mon, Oct 26, 2020 at 04:52:12PM -0700, Matt Votava wrote:
It appears the tests are broken for python 3.7.8, possibly due to Martiusweb/asynctest#152. Should I set doCheck false for python 3.7.8?
As these packages are exclusively used in home-assistant and that runs
on python 3.8, go ahead and disable them for 3.7.
… --
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#101557 (comment)
|
I just want to check, disable the whole package, or just the tests? I disabled the package now. If that is what you meant should I change the commit message to python38Packages? |
As the package does not seem to work >3.7.8 I don't really care if we disable the package or the tests. python3Packages are currently synonym for python38Packages, so both are fine. |
Thanks! |
Motivation for this change
Home Assistant update broke ZHA component by adding a new zigpy module zigpy-znp.
Things done
Init package pythonPackages.zigpy-znp.
This requires updating zigpy, and the other zigpy python modules.
I have checked to make sure this works on my H-A server.
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)