Github user ran-z commented on a diff in the pull request:

    https://github.com/apache/incubator-ariatosca/pull/95#discussion_r111150551
  
    --- Diff: aria/modeling/service_instance.py ---
    @@ -529,6 +524,32 @@ def validate_capabilities(self):
                     satisfied = False
             return satisfied
     
    +    def find_host(self):
    +        def _find_host(node):
    --- End diff --
    
    im still a bit confused by this myself - caching the host is definitely a 
good idea, but it looks here like the internal method _find_host is called 
recursively repeatedly.
    I'm not sure this is exactly the same as what Maxim meant, but I think I'd 
expect each node's host finding method to run once at most, so that would mean 
either "host" is a self-initializing property, or a field initialized at its 
constructor, or you could use the "find_host" method, but basically what i'm 
missing here is the check for whether the value has been cached already or not.
    
    i'd expect it to be more like:
    ```
    def find_host(self):
        def _find_host(node):
        ....
            host = the_relationship.target_node.find_host()
        ....
    
    if not self.host:  # or indeed use some extra flag
        self.host = _find_host(node)    
    return self.host
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to