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

appdaemon: fix astral dependency #116777

Merged
merged 1 commit into from Mar 20, 2021

Conversation

mbrgm
Copy link
Member

@mbrgm mbrgm commented Mar 18, 2021

Motivation for this change

appdaemon v4.0.5 pins astral to v1.10.1. The appdaemon package
definition removes this pinning. However, the current nixpkgs version of
astral (v2.x) is incompatible with the v1.x API. I added an override for
the astral package to bring back v1.10.1 into this derivation.

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.

Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

The line --replace "astral==1.10.1" "astral" \ can be removed.

@@ -6,6 +6,14 @@
let
python = python3.override {
packageOverrides = self: super: {
astral = super.astral.overridePythonAttrs (oldAttrs: rec {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a link to an upstream issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is no upstream issue. The derivation introduces the regression by unpinning the astral package version. See the commit message for detailed information. Here's a traceback of the symptom:

Mar 17 00:38:00 nixos appdaemon[9134]: Traceback (most recent call last):
Mar 17 00:38:00 nixos appdaemon[9134]:   File "/nix/store/la71x6y3npq1fzrvkv2f7kb481c2yls4-appdaemon-4.0.5/lib/python3.8/site-packages/appdaemon/__main__.py", line 117,>
Mar 17 00:38:00 nixos appdaemon[9134]:     self.AD = ad.AppDaemon(self.logging, loop, **appdaemon)
Mar 17 00:38:00 nixos appdaemon[9134]:   File "/nix/store/la71x6y3npq1fzrvkv2f7kb481c2yls4-appdaemon-4.0.5/lib/python3.8/site-packages/appdaemon/appdaemon.py", line 187>
Mar 17 00:38:00 nixos appdaemon[9134]:     self.sched = scheduler.Scheduler(self)
Mar 17 00:38:00 nixos appdaemon[9134]:   File "/nix/store/la71x6y3npq1fzrvkv2f7kb481c2yls4-appdaemon-4.0.5/lib/python3.8/site-packages/appdaemon/scheduler.py", line 62,>
Mar 17 00:38:00 nixos appdaemon[9134]:     self.init_sun()
Mar 17 00:38:00 nixos appdaemon[9134]:   File "/nix/store/la71x6y3npq1fzrvkv2f7kb481c2yls4-appdaemon-4.0.5/lib/python3.8/site-packages/appdaemon/scheduler.py", line 220>
Mar 17 00:38:00 nixos appdaemon[9134]:     self.location = astral.Location(("", "", latitude, longitude, self.AD.tz.zone, elevation))
Mar 17 00:38:00 nixos appdaemon[9134]: AttributeError: module 'astral' has no attribute 'Location'

appdaemon v4.0.5 pins astral to v1.10.1. The appdaemon package
definition removes this pinning. However, the current nixpkgs version of
astral (v2.x) is incompatible with the v1.x API. I added an override for
the astral package to bring back v1.10.1 into this derivation.
@mbrgm mbrgm force-pushed the fix-appdaemon-astral-dependency branch from c9b02ca to 1b8bc64 Compare March 19, 2021 09:27
@ofborg ofborg bot requested a review from dotlambda March 19, 2021 09:37
@dotlambda
Copy link
Member

Can we use the patches from AppDaemon/appdaemon#1168 instead?

@mbrgm
Copy link
Member Author

mbrgm commented Mar 20, 2021

Can we use the patches from AppDaemon/appdaemon#1168 instead?

Sure... however upstream author is planning to release a new version soon (AppDaemon/appdaemon#1217) and my motivation to rebase the patch you mentioned and re-author this PR is quite low regarding it would only be effective for a few days and I have a solution in place already. So I'd either stay with this fix or close the PR to wait for the new appdaemon version.

@dotlambda dotlambda merged commit a04bdd8 into NixOS:master Mar 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants