Reloading logic of global modules seems broken

[Big4SMK]

Using appdaemon 4.0.5, I'm having trouble with reloading global modules. Here's a fairly straight forward testcase. I'm building a apple that depends on fruit. This works as expected the first time, but when I change TEST_VAR in fruit.py, things start to go out of sync.

apps.yaml

global_modules:
  - fruit
apple_app:
  module: apple_module
  class: AppleClass

fruit.py

class Fruit:
        TEST_VAR = "1234"

        def test_func(self):
                self.log(f"test_func:  self.TEST_VAR = {self.TEST_VAR}")
                self.log(f"test_func: Fruit.TEST_VAR = {Fruit.TEST_VAR}")

apple_module.py

import appdaemon.plugins.hass.hassapi as hass
import fruit

class AppleClass(hass.Hass, fruit.Fruit):
        def initialize(self):
                self.depends_on_module(fruit)
                self.log(f"AppleClass.initialize:  self.TEST_VAR = {self.TEST_VAR}")
                self.log(f"AppleClass.initialize: Fruit.TEST_VAR = {fruit.Fruit.TEST_VAR}")
                self.test_func()

Reloading appdaemon will yield:

2021-01-25 10:08:34.661934 INFO apple_app: test_func: Fruit.TEST_VAR = 1234
2021-01-25 10:08:34.661440 INFO apple_app: test_func:  self.TEST_VAR = 1234
2021-01-25 10:08:34.660806 INFO apple_app: AppleClass.initialize: Fruit.TEST_VAR = 1234
2021-01-25 10:08:34.660316 INFO apple_app: AppleClass.initialize:  self.TEST_VAR = 1234
2021-01-25 10:08:34.535455 INFO AppDaemon: Initializing app apple_app using class AppleClass from module apple_module
2021-01-25 10:08:34.525045 INFO AppDaemon: Loading Global Module: /home/sander/homeassistant/appdaemon/apps/fruit.py
2021-01-25 10:20:42.580018 INFO AppDaemon: Loading App Module: /home/sander/homeassistant/appdaemon/apps/apple_module.py

Now if I change TEST_VAR in fruit.py to "12345", appdaemon yields:

2021-01-25 10:17:35.199751 INFO apple_app: test_func: Fruit.TEST_VAR = 12345
2021-01-25 10:17:35.199248 INFO apple_app: test_func:  self.TEST_VAR = 1234
2021-01-25 10:17:35.197808 INFO apple_app: AppleClass.initialize: Fruit.TEST_VAR = 12345
2021-01-25 10:17:35.196653 INFO apple_app: AppleClass.initialize:  self.TEST_VAR = 1234
2021-01-25 10:17:35.192638 INFO AppDaemon: Initializing app apple_app using class AppleClass from module apple_module
2021-01-25 10:17:35.190925 INFO AppDaemon: Reloading Module: /home/sander/homeassistant/appdaemon/apps/fruit.py

You see fruit.py is being reloaded, but the apple_app is only reinitialized instead of reloaded, even though it is depending on fruit per the first line of initialize. Restarting appdaemon gets things back on track again, as both files will be (re)loaded instead of reinitialized.

Changing apps.yaml like below to signal the global dependency will yield the same erroeous behavior.

apple_app:
  module: apple_module
  class: AppleClass
  global_dependencies:
    - fruit

[ReneTode]

i think thats because you use different options to do the same.

in the yaml it should be:

global_modules:
  - fruit
apple_app:
  module: apple_module
  class: AppleClass
  global_dependencies:
    - fruit

that should be enough to restart the app when the fruit module has changed. and in the apple app:

import appdaemon.plugins.hass.hassapi as hass
import fruit

class AppleClass(hass.Hass):
        def initialize(self):
                # self.depends_on_module(fruit) # not needed because you use global module
                self.log(f"AppleClass.initialize:  self.TEST_VAR = {self.TEST_VAR}")
                self.log(f"AppleClass.initialize: Fruit.TEST_VAR = {fruit.Fruit.TEST_VAR}")
                self.test_func()

[Big4SMK]

that should be enough to restart the app when the fruit module has changed.

No, it is not, that is the whole point. I'm sorry if I was unclear in my previous post, but I am seeing the described issue of my apple_app NOT being reloaded in BOTH of the following two cases:

1. Setting the dependency in the AppleClass by setting self.depends_on_module(fruit)
2. Setting the dependency in apps.yaml by setting

global_dependencies: 
  - fruit

Can you post the logs of it working for you?

[ReneTode]

test.yaml

global_modules:
  - fruit
apple_app:
  module: apple_app
  class: AppleClass
  global_dependencies:
    - fruit

apple_app.py

import appdaemon.plugins.hass.hassapi as hass
import fruit

class AppleClass(hass.Hass):
        def initialize(self):
                self.log(f"AppleClass.initialize: Fruit.TEST_VAR = {fruit.Fruit.TEST_VAR}")

fruit.py

class Fruit:
        TEST_VAR = "1234"

        def test_func(self):
                self.log(f"test_func:  self.TEST_VAR = {self.TEST_VAR}")

logs after changing fruit.py

2021-01-25 13:33:59.327650 INFO apple_app: AppleClass.initialize: Fruit.TEST_VAR = 12345
2021-01-25 13:33:59.316981 INFO AppDaemon: Initializing app apple_app using class AppleClass from module apple_app
2021-01-25 13:33:59.308492 INFO AppDaemon: Reloading Module: /mnt/usbdrive/pi/appdaemon/AD_controlboard/apps/fruit.py
2021-01-25 13:33:59.304297 INFO AppDaemon: Terminating apple_app
2021-01-25 13:33:28.091520 INFO apple_app: AppleClass.initialize: Fruit.TEST_VAR = 1234
2021-01-25 13:33:28.080002 INFO AppDaemon: Initializing app apple_app using class AppleClass from module apple_app
2021-01-25 13:33:28.073163 INFO AppDaemon: Reloading Module: /mnt/usbdrive/pi/appdaemon/AD_controlboard/apps/apple_app.py

so it works.

[Big4SMK]

Hi ReneTode. I appreciate your effort, but unfortunately you have picked the piece of my code that I showed to work too. I'd like to see your log for the piece that doesn't work on my end. My log showed:

2021-01-25 10:17:35.199751 INFO apple_app: test_func: Fruit.TEST_VAR = 12345
2021-01-25 10:17:35.199248 INFO apple_app: test_func:  self.TEST_VAR = 1234
2021-01-25 10:17:35.197808 INFO apple_app: AppleClass.initialize: Fruit.TEST_VAR = 12345
2021-01-25 10:17:35.196653 INFO apple_app: AppleClass.initialize:  self.TEST_VAR = 1234
2021-01-25 10:17:35.192638 INFO AppDaemon: Initializing app apple_app using class AppleClass from module apple_module
2021-01-25 10:17:35.190925 INFO AppDaemon: Reloading Module: /home/sander/homeassistant/appdaemon/apps/fruit.py

Please note both Fruit.TEST_VAR lines give indeed the right answer "12345" after fruit.py was reloaded. What doesn't work is the variables that use self.TEST_VAR, as that TEST_VAR is part of the AppleClass class by inheritance.

Could you please run my full example and show me the logs?

[ReneTode]

thats obvious, and it would and could never work.

the AppleClass is never reloaded. reloading the app means that the initialise function is reloaded. so the inheritance stays the same.

so im sorry but you cant have both inheritance and automaticly reloading.

so thats not broken, but it never worked that way.

[Big4SMK]

Your definition of "reload" seems to be given by what appdaemon does, not by what I'd argue is the natural thing to expect: an importlib.reload().

To me it's far from obvious why that reload "would and could never work", I'm hoping you can explain in a bit more detail how calling importlib.reload(apple_module) is impossible if a importlib.reload(fruit) is done when the file changes?

[ReneTode]

the global module is reloaded. and the app is reinitialised, and not reloaded when the global module changes.

thats how it always worked and thats how its described in the docs.

and i say that it cant work because of the way appdaemon is build, i didnt say that its impossible to change appdaemon. you could make a feature request to change that AD reloads the class from an app instead of reinitialising.

i dont know if it would be possible within the actual AD structure, and how much work would be involved, and if and when it would be changed.

im just telling that it works as it is described and intended. so its not broken.

[ReneTode]

its by the way probably causing breaking changes and a lot of trouble when that module would be reloaded instead of the app reinitialised.

the app module could also contain other classes that would be used for other apps. reloading the module would cause that all those classes and apps, that are not depending on the global module would also be reinitialised.

so reloading the module instead of reinitialising the app could cause a chain of unexpected app reloads, and even loops.

[Big4SMK]

It sounds like it all comes down to how one should interpret the following statement from the docs:

The dependency directive will identify the name of the App it cares about, and AppDaemon will see to it that the dependency is loaded before the App depending on it, and that the dependent App will be reloaded if it changes.

My expectation from "will be reloaded" is something like an importlib.reload(), your expectation is "the app is reinitialised, and not reloaded". Let's agree to disagree :-)

Is there a way for me to mark this as a feature request?

[acockburn]

You just did ;) Hope you are well Sander, nice to see you over here :)

I’ll double check what the actual intention was here - it seems to make sense to reload the global module to account for any changes in it, without that behavior I don’t see the point.

[ReneTode]

the global module is reloaded Andrew. the problem he has is that the module that contains the app isnt reloaded.

which is impossible to change, because when the module that contains the class from the app that depends on the global module, can also contain many other classes, which could depend on other modules, or even the same. reloading the whole module that contains the app that depends on a module, would cause enormous problems.

and i agree @Big4SMK that when you look at it your way, the words: "App will be reloaded" can let you think something else then what is really happening. its semantics, but for those who are familiar with appdaemon its obvious, for those who come from a python background i guess its interpreted different.

and like i stated in my second response (and here to andrew) the feature request you ask for will cause extreme problems when people have more then 1 class in a module that belong to apps. it would only be possible if AD would state that modules only can contain 1 class. and all that have more classes in 1 module would have to change their modules.

and it would only be to make that others could use self.something, instead off fruit.something, so i would really advise strongly against actions to make that happen.

[mementum]

Hi guys,

Before opening a ticket I searched through the existing issues and this one matches 100% my "problem"

Understanding the arguments against reloading a "module" as opposed to reloading an "app" from a module, would the following be a potential way for it:

• Use apps.yaml to declare modules which can be reloaded (i.e.: will be recompiled and reimported) if an "app" declared in the module has to be reloaded due to a dependency.

In my personal case I define at most 1 class "app" in a module and have no intention to do otherwise. I could easily mark all modules as "reloadable"

It's probably a long shot and I guess requires changes to the architecture and logic which are not worth the effort and risk of breaking something.

Best regards

[Justihar]

Hmm.. I thought someone had put in a PR for this not too long ago that fixes the issue but maybe hasn't been approved yet.

Hi guys,

Before opening a ticket I searched through the existing issues and this one matches 100% my "problem"

Understanding the arguments against reloading a "module" as opposed to reloading an "app" from a module, would the following be a potential way for it:

[mementum]

Here it is: #1477

[Justihar]

Yeah, there you go, totally forgot about that. I'm sure it will be fixed at some point. :)

[jaycrossler]

Thanks for the discussion on this, I now understand more the behaviors I am seeing (and I also am is-assumed definition of the reload word).

Would another approach be to update either documentation or a best-practices guide for how to develop with AppDaemon when you have multiple classes? I’m finding my workflow is much slower and thus I’m letting bad behaviors creep in (putting all classes in one file, over-logging, not enforcing logical separation, etc). For example, do people have a way to use PyCharm or an IDE within AppDaemon to get visibility into processing and working with multiple classes? I saw one example when searching but it doesn’t seem scalable.

[ReneTode]

if you think the docs can be better, then your always welcome to say how, or to make the efford an make a PR. the principle from AD was always: keep it simple, keep it small, make it reusable. python makes it possible to put everything in 1 file, but in AD its better to have a lot of small files. the easiest way it to adchieve 1 class in 1 file, because 1 app is 1 class.

[acockburn]

It's been a long time coming but this is fixed in 4.2.3 as well as the addition of a new way of specifying global modules that allow for nested dependencies.