| claudiupopa | smoser, Odd_Bloke: any reason why cloudinit.logging is called `logging`? | 12:16 |
|---|---|---|
| claudiupopa | Are we expecting it to always replace the builtin logging module? | 12:16 |
| openstackgerrit | Scott Moser proposed stackforge/cloud-init: add unregister and reset to DictRegistry and use https://review.openstack.org/209696 | 15:16 |
| smoser | Odd_Bloke, https://review.openstack.org/#/c/209696/ | 15:17 |
| smoser | it is nonobvious to me how to test the udpate_configuration | 15:17 |
| smoser | could you quickly add 2 ? | 15:18 |
| Odd_Bloke | smoser: 2? | 15:18 |
| smoser | id ont know why i said 2 | 15:18 |
| smoser | i guess 3 | 15:19 |
| Odd_Bloke | smoser: Oh, add some tests? | 15:19 |
| claudiupopa | why isn't the reset separated? | 15:19 |
| smoser | one for reset, update and update_with_null | 15:19 |
| smoser | claudiupopa, as in you want to reset with no config | 15:19 |
| smoser | right? | 15:19 |
| claudiupopa | yeah. | 15:19 |
| claudiupopa | I tend to not like boolean flags that modifies something. | 15:20 |
| claudiupopa | I mean functions with boolean flags. | 15:20 |
| claudiupopa | Just a question, is expected from DictRegistry to be thread safe? | 15:21 |
| Odd_Bloke | claudiupopa: No. | 15:27 |
| Odd_Bloke | claudiupopa: But only because there was no reason to at this point, rather than from some deep opposition to thread-safe data structures. :p | 15:28 |
| smoser | claudiupopa, i think i'd just drop the 'reset' | 15:33 |
| smoser | and if you want reset, you just instantiated_handler_registry.reset() | 15:34 |
| claudiupopa | smoser: yeah, that would be better imo. | 15:34 |
| Odd_Bloke | smoser: If you still want me to add tests after that, let me know when I can pull the change to play with it. :) | 15:41 |
| smoser | Odd_Bloke, if you could, i'd really appreciate it. | 15:42 |
| Odd_Bloke | smoser: Cool, give me a shout when it's ready. | 15:43 |
| openstackgerrit | Scott Moser proposed stackforge/cloud-init: add unregister and reset to DictRegistry and use https://review.openstack.org/209696 | 15:44 |
| smoser | Odd_Bloke, ^ | 15:44 |
| claudiupopa | by the way. any reason why cloudinit.logging is called `logging`? | 15:46 |
| Odd_Bloke | smoser: ^ | 15:48 |
| smoser | to be like loggin | 15:50 |
| smoser | logging | 15:50 |
| claudiupopa | So we'll use that instead of the builtin logging module? | 15:50 |
| smoser | yeah. by name it is the same and the intent is it looks the same. | 15:51 |
| claudiupopa | Because in that case, it should be made explicit in url_helper and templater. | 15:51 |
| claudiupopa | as in from cloudinit import logging instead of a plain `import logging`. | 15:51 |
| claudiupopa | Since absolute import is not activated in those modules. | 15:52 |
| smoser | you're right. those should use tcloudinit.logging | 15:52 |
| smoser | at least that is the intent. | 15:52 |
| smoser | good catch claudiupopa | 15:52 |
| Odd_Bloke | smoser: Going in to a meeting now; will look at those tests in ~30 minutes. | 16:00 |
| smoser | k. | 16:05 |
| openstackgerrit | Claudiu Popa proposed stackforge/cloud-init: Add an API for loading a data source https://review.openstack.org/209520 | 16:56 |
| claudiupopa | Odd_Bloke, smoser ^ still work in progress, but I'll appreciate a review regarding the direction. | 16:58 |
| smoser | so as you suggest4ed, https://review.openstack.org/#/c/209520/2/cloudinit/plugin_finder.py | 16:59 |
| smoser | shoudl have | 16:59 |
| smoser | from . import logging | 16:59 |
| smoser | right ? | 16:59 |
| claudiupopa | Yeah, I noticed right now that. | 17:00 |
| claudiupopa | Thanks. | 17:00 |
| smoser | https://review.openstack.org/#/c/209520/2/cloudinit/sources/openstack/httpopenstack.py | 17:03 |
| smoser | can we jsut tlak here.. ? | 17:03 |
| claudiupopa | yeah. | 17:03 |
| openstackgerrit | Claudiu Popa proposed stackforge/cloud-init: Use an explicit absolute import for importing the logging module https://review.openstack.org/210035 | 17:03 |
| openstackgerrit | Daniel Watkins proposed stackforge/cloud-init: Add unregister and reset to DictRegistry and use https://review.openstack.org/209696 | 17:04 |
| smoser | so there, data_sources() does not know if it should do network data sources or local ? | 17:05 |
| Odd_Bloke | smoser: I think it just needed the one test, which I've pushed up. | 17:05 |
| claudiupopa | No, not at that point. | 17:05 |
| claudiupopa | That was the intent of the strategies. | 17:05 |
| claudiupopa | Which is a fancy word for implementing filters. | 17:06 |
| claudiupopa | The idea is that if you want the network data sources only or any other kind of data source, you implement a strategy and you pass it to get_data_source. | 17:06 |
| Odd_Bloke | Now I'm EOD'ing. :) | 17:06 |
| smoser | Odd_Bloke, htanks | 17:06 |
| claudiupopa | I did it in this way since it really separates the concerns. | 17:07 |
| claudiupopa | Each data source will not be interested in how it will be chosen. | 17:07 |
| smoser | claudiupopa, ok, i think thats probably fine, but the example strategy of 'serial' implied to me 'parallel' | 17:08 |
| claudiupopa | yep. | 17:08 |
| claudiupopa | At that point you can combine network + parallel. | 17:08 |
| claudiupopa | Or any other combination you like | 17:08 |
| claudiupopa | https://review.openstack.org/#/c/209520/2/cloudinit/sources/base.py | 17:09 |
| claudiupopa | check valid_data_sources. | 17:09 |
| smoser | i think that is ok. ... but i want to be clear somehow when loading the datasource to search | 17:09 |
| smoser | that it does not have (or is not guaranteed) functional network | 17:09 |
| smoser | ie, the network could be wrong at this poitn an dit shoudlnt consider using it | 17:09 |
| claudiupopa | yep, I see your point. Probably the actual strategies that will be used will be specific per each step in the stages. | 17:10 |
| smoser | claudiupopa, thats the real extent of my comemnts then i think | 17:28 |
| claudiupopa | no other obvious issue? | 17:29 |
| claudiupopa | thanks! | 17:31 |
| claudiupopa | I'll push the tests tomorrow. | 17:31 |
| smoser | claudiupopa, can you https://review.openstack.org/#/c/209696/ ? | 17:32 |
| smoser | just as it is mine, i think you agree with it at this point. | 17:32 |
| smoser | can you push yes for workflow ? | 17:32 |
| claudiupopa | yep, looks nice. | 17:33 |
| openstackgerrit | Merged stackforge/cloud-init: Add unregister and reset to DictRegistry and use https://review.openstack.org/209696 | 17:46 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!