Wednesday, 2016-08-03

*** spzala has quit IRC00:56
*** spzala has joined #openstack-heat-translator01:24
*** spzala has quit IRC01:27
*** spzala has joined #openstack-heat-translator01:27
*** bobh has joined #openstack-heat-translator01:44
*** bobh has quit IRC02:09
*** spzala has quit IRC03:39
openstackgerritSanthosh64 proposed openstack/heat-translator: Implemented Scaling policies in heat translator  https://review.openstack.org/30263605:39
*** spzala has joined #openstack-heat-translator05:40
*** spzala has quit IRC05:44
*** spzala has joined #openstack-heat-translator12:25
*** zhipeng has joined #openstack-heat-translator14:02
*** zhipeng has quit IRC14:06
*** zhipeng has joined #openstack-heat-translator14:07
*** zhipeng has quit IRC14:42
*** zhipeng has joined #openstack-heat-translator14:43
*** bobh has joined #openstack-heat-translator14:43
*** bobh has quit IRC14:49
*** zhipeng has quit IRC16:27
*** zhipeng has joined #openstack-heat-translator16:28
*** bobh has joined #openstack-heat-translator16:46
*** bobh has quit IRC16:51
*** openstackgerrit_ has joined #openstack-heat-translator17:06
*** openstackgerrit_ has quit IRC17:08
*** shangxdy has joined #openstack-heat-translator17:11
shangxdyHI, spzala17:11
*** spzala has quit IRC17:12
*** spzala has joined #openstack-heat-translator17:20
*** spzala has quit IRC17:20
*** spzala has joined #openstack-heat-translator17:20
spzalashangxdy: Hi17:22
spzalashangxdy: up late :-)17:23
shangxdy:)17:24
shangxdyI have just now notice your comments.17:24
spzalashangxdy: sure, does they make sense?17:24
spzalashangxdy: I have just right now posted a comment17:24
spzalashangxdy: drafted it but just published.. please take a look17:25
spzalashangxdy: pointing exactly where in code I think we should be able to handle sub_mapping17:25
shangxdyI agree code must be tested enough17:25
shangxdyOk17:26
spzalashangxdy: thanks, but please look at my comment I just posted.17:26
shangxdy"only setting sub_mapping to null in topology_template for now (which is null right now anyway) and build passes successfully"17:26
shangxdyAre you sure to update the patch completely before your test?17:27
spzalashangxdy: with that comment what I was saying is, if you remove all the changes you  have in tosca_template.py that doesn't impact the build17:27
spzalaexcept that topology_template takes sub_map in arg which is set to null even with your changes I think..17:28
spzalaAs I commented, I would alway like to have patch backed by tests but for this patch17:28
shangxdyI'll update and have a look17:28
spzalashangxdy: cool, per my latest comment, I think it should be easy and proper way to handle sub_map by looking into imports and finding out which ones has sub_mapping based on keyword17:29
spzalano need to modify constructor or iteratively call the ToscaTemplate class from within it17:29
shangxdyDo you notice the test function17:30
shangxdy Do you notice the test function17:30
shangxdyDo you notice the test function test_system_template(self) in test_topology_template.py?17:30
spzalaalso, I was saying that the method you have added in tosca_template.py are not tested and has no impact, you can remove them for now from this patch and add them with tests that you have already planned for17:31
spzalayes I did17:31
spzalabut that should pass jenkins unless I made some local changes and that made tox pass :(17:32
spzalabut if anything that I missed and is needed to pass the test method, I am totally fine with it17:32
shangxdyI have test part of the patch, and if you set sub_mapping to none, the test function will run error17:33
spzalamy main concerned is modifying the constructor17:33
spzalado you think my latest comment is something you can incorporate ?17:34
shangxdyYour test is  main about the constructor without extra param?17:34
spzalafor sub_mapping the time it makes sense is when the imported templates has that section17:35
spzalaand in that we substitute node in main template which is passed to the ToscaTemple17:35
spzalaso that's the main case that needs to be handled17:35
spzalayup that's my main concern17:36
spzalaiterating over template imports it should be easy to get which one is meant for sub_mapping17:37
shangxdy"a method here to get all the templates" the all templates means nested templates?17:38
spzalaall the templates that offers substitution_mapping17:39
spzalaI believe that's what you need right?17:40
spzalaonce you have that, then I guess you can use that with many of your current methods you have17:40
shangxdyI think so, but now the patch find the nested templates too, they are only instantiated before matching the  node template17:41
spzalayou can use the same method to find all nested templates as well, again you need to iterate over imports17:42
spzalaso nested templates are superset and one with sub_mappings are subset of it17:42
spzalasince it's a big patch I would usually find it more useful and test if we iteratively add patches as needed17:43
spzalaso if we can handle substitute mappings that should enable initial support for it17:44
spzalaand you can add more on top of it as needed17:44
shangxdy"iterating over template imports" is based on the raw yaml format?17:45
shangxdyOk, we can import the nested template first. It's sure too big patch17:45
spzalaonce we load templates, we have a method in Imports.py that iterates over list of imports ("importslist" var)17:47
*** bobh has joined #openstack-heat-translator17:47
shangxdyI may not express my whole idea completely in the patch.17:47
*** zhipeng has quit IRC17:47
spzalai guess that gives all the nested (imports) templates17:48
spzalashangxdy: sure, so if you can only provide support for sub_mappings with the current test templates you have modified and provide tests for validating them and substitute that would be great17:49
spzalashangxdy: and by all mean, it's a great work .. the sub_mappings.py you have added is great17:49
shangxdyI think it's no problem to get all the imported templates, the point is to associate them  with node templates.17:50
spzalaso if you can get list of all templates that supports sub_mapping before call to the topology_templates it should be in similar line to with what you have17:51
*** bobh has quit IRC17:52
spzalathat's17:52
spzalaif self.topology_template.tpl:17:52
spzalayou get all templates that provides sub_mapping17:52
spzalaand then you already have call later where you use it,17:52
spzalareturn TopologyTemplate(self._tpl_topology_template(),17:52
spzala                                self._get_all_custom_defs(),17:52
spzala                                self.relationship_types,17:52
spzala                                self.parsed_params,17:52
spzala                                self.sub_mapped_node_template)17:52
spzalahopefully you don't need to change flow much17:53
shangxdyIn ToscaTemplate create all TopologyTemplate objects? not nested toscatemplate objects?17:56
shangxdyI thought doubt It, but this solution will miss some informations such as repo.17:58
spzalano I am saying that once you get all the template which offers sub_mappings17:58
spzalayou should be able to go with your flow without nested toscatemplate objects17:59
spzala(we should avoid modifying constructor and nested toscatemplate objects)18:00
shangxdyHow to get all the nested template except the solution in the patch?18:01
shangxdyI know your concern "avoid modifying constructor":)18:02
spzala:) ok, let's try one more time or I will try to modify code:18:02
spzala1. get all the templates that offers sub_mappings18:02
shangxdyfine, can you give the modified code about that?18:04
spzalaOK, let me try working on it and I will get in touch with you18:04
spzalaI will be starting to work on tosca parser release so please bear with me if I take some time18:05
spzalawhen are you going on vacation?18:05
spzalaor are you already on vacation? :)18:06
shangxdy:) I'll try to avoid modifying the constructor, but i give up because of diffculty18:06
spzalashangxdy: :) no worries, if you can try on that please put some thoughts, and meanwhile I will start modifying the patch locally and see how it goes18:07
shangxdyThe difficulty come from much code modified and heat translator.18:08
spzalashangxdy: agree it's difficult but let's try, once we modify the signature of constructor and make a release we are stuck with it due to backward compatibility reason18:09
shangxdyWith current patch, heat-translator will be easy to support sub_mapping.18:09
spzalashangxdy: sure, my attempt is to keep current flow much same as possible, but adding arg when it's not needed that's something we need to avoid18:10
spzalafor a method signature it's ok, but for an entry class it's not unless we can't find any other way18:11
shangxdyOk, let's try, i'll do it again after vacation.18:11
spzalashangxdy: ok, perfect18:11
spzalashangxdy: thanks for your patience :)18:11
spzalaand have a great vacation time18:11
*** bobh has joined #openstack-heat-translator18:11
shangxdy:) thanks your patience too.18:12
spzalashangxdy: :) no problem18:12
shangxdyenjoy family vacation:)18:12
spzalashangxdy: :) thanks, you too!18:13
spzalashangxdy: bye18:13
shangxdybye:)18:13
spzala:)18:13
*** shangxdy has quit IRC18:14
*** spzala has quit IRC22:08
*** spzala has joined #openstack-heat-translator22:08
*** spzala has quit IRC22:13
*** spzala has joined #openstack-heat-translator22:35
*** spzala has quit IRC22:38
*** spzala has joined #openstack-heat-translator22:38
*** bobh has quit IRC22:47
*** bobh has joined #openstack-heat-translator23:52
*** spzala has quit IRC23:55

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!