| smoser | blackboxsw, we will. none of those are terribly important. the ppc64 breakage isn't present in the SRU'd version. | 01:41 |
|---|---|---|
| smoser | i dont think | 01:41 |
| === shardy_afk is now known as shardy | ||
| === shardy is now known as shardy_mtg | ||
| === shardy_mtg is now known as shardy | ||
| === shardy is now known as shardy_mtg | ||
| === shardy_mtg is now known as shardy | ||
| smoser | blackboxsw, i should have noticed / thought of this before, but by doing the json schema doc | 16:42 |
| smoser | we lose | 16:42 |
| smoser | python3 | 16:42 |
| smoser | >> from cloudinit.config import cc_bootcmd | 16:43 |
| smoser | >> help(bootcmd) | 16:43 |
| smoser | er... help(cc_bootcmd) | 16:43 |
| smoser | right ? | 16:43 |
| blackboxsw | hrm right smoser | 17:00 |
| blackboxsw | sorry was in another window | 17:00 |
| blackboxsw | good thought, I bet we can fix that | 17:01 |
| blackboxsw | yeah I can fix that. smoser WDYT? http://paste.ubuntu.com/25491373/ | 17:04 |
| smoser | blackboxsw, http://paste.ubuntu.com/25491366/ | 17:04 |
| smoser | that was mine | 17:04 |
| blackboxsw | haha | 17:04 |
| smoser | yours prettier | 17:05 |
| blackboxsw | yeah we already have get_schema_doc. no need for a 2nd function :) | 17:05 |
| blackboxsw | I can fold that into the cc_bootcmd/resizefs branch (and fix runcmd/ntp as well) | 17:06 |
| smoser | get_schema_doc modifies schema | 17:06 |
| blackboxsw | ahh true, could deepcopy | 17:07 |
| smoser | the other issue with your solutin there is it is slow. i think | 17:07 |
| smoser | i think we then hit usage of yaml on import of cc_bootcmd | 17:07 |
| smoser | so i dont know. | 17:08 |
| blackboxsw | lemme see how yours outputs | 17:08 |
| smoser | not anywhere near as nice. | 17:08 |
| smoser | and i'd have used yorus if i had know of that | 17:09 |
| blackboxsw | right, but at least it's something (because I'd like to trim the module docstr to a single line of text in all modules w/ jsonschema) | 17:09 |
| * blackboxsw really wonders how much 'support' we need to worry about for help() callers in python shell | 17:10 | |
| blackboxsw | I'd be inclined to use your markup to meet the need if it arises (but avoid the cost of templating etc on module load) | 17:10 |
| smoser | blackboxsw, | 17:15 |
| smoser | yaml_string = yaml.dump(property_dict.get('enum')).strip() | 17:15 |
| smoser | property_type = yaml_string[1:-1].replace(', ', '/') | 17:15 |
| smoser | is there a reason not : | 17:15 |
| smoser | property_type = '/'.join(property_dict['enum']) ? | 17:15 |
| blackboxsw | smoser: for docs I wanted the actual strings rendered to match what we expect users to write in a yaml file | 17:17 |
| blackboxsw | 'true/false' instead of True/False | 17:17 |
| smoser | ah. that does make sense. | 17:17 |
| blackboxsw | but doesn't yaml accept both? | 17:18 |
| blackboxsw | Regexp: | 17:18 |
| blackboxsw | y|Y|yes|Yes|YES|n|N|no|No|NO | 17:18 |
| blackboxsw | |true|True|TRUE|false|False|FALSE | 17:18 |
| blackboxsw | |on|On|ON|off|Off|OFF | 17:18 |
| blackboxsw | meh, so maybe it doesn't matter http://yaml.org/type/bool.html | 17:18 |
| blackboxsw | heh not fully supported | 17:22 |
| blackboxsw | http://pastebin.ubuntu.com/25491483/ | 17:22 |
| blackboxsw | some of the regexps don't seem to match. better to be safe/strict | 17:22 |
| smoser | http://paste.ubuntu.com/25491485/ | 17:23 |
| blackboxsw | +1 simple and efficient | 17:24 |
| blackboxsw | I'll add that smoser | 17:24 |
| blackboxsw | better not to spec cycles on yaml.load if all we want is a simple dict lookuo | 17:24 |
| blackboxsw | better not to spec cycles on yaml.load if all we want is a simple dict lookup | 17:24 |
| smoser | blackboxsw, name 'ymap' as "_YAML_AS_STRING" or something and put it at top of that | 17:25 |
| blackboxsw | yeah w/ a oneline comment explaining the intent | 17:26 |
| blackboxsw | will do | 17:26 |
| smoser | then, we're fairly close to not even importing yaml | 17:27 |
| smoser | except on checking | 17:27 |
| blackboxsw | could separate that to a different module | 17:28 |
| blackboxsw | hrm, also in some of the other doc generation we have yaml.dump calls | 17:30 |
| blackboxsw | like rendering schema['examples'] | 17:30 |
| smoser | i think that was the only other one | 17:36 |
| smoser | right? | 17:36 |
| smoser | blackboxsw, ^ | 17:52 |
| smoser | i'm not sure why wer'e using yaml to dump the examples, which are already formatted strings | 17:52 |
| smoser | oh. i see. | 18:14 |
| blackboxsw | sorry was running an errand. | 18:20 |
| blackboxsw | smoser: yeah I had allowed for either md formatted strings or python dict objects | 18:21 |
| blackboxsw | probably something that we don't really have to support and we could drop the yaml.dumps stuff | 18:21 |
| blackboxsw | I had thought we might want to leverage examples automated unittests, and having a examples in dict format already provided would make it easy to pass into some automated handle tests. | 18:22 |
| blackboxsw | but, we could just as easily have the unit tests yaml.load(example_content_string) | 18:23 |
| blackboxsw | if we every go down that route in unittests | 18:23 |
| blackboxsw | s/every/ever/ | 18:23 |
| smoser | http://paste.ubuntu.com/25491809/ | 18:23 |
| blackboxsw | I'm glad you agree :) and did the work | 18:23 |
| blackboxsw | thanks | 18:24 |
| smoser | hardest part was making flake8 happy | 18:24 |
| blackboxsw | I like it | 18:25 |
| smoser | and then we're not far from having yaml only necessary at all in the schema path when validating a cloud-config | 18:26 |
| smoser | specifically *not* when validating a dictonary | 18:26 |
| smoser | (i realize we have yaml everywhere, so not a big deal) | 18:27 |
| blackboxsw | I'm not even sure we need the else: | 18:27 |
| blackboxsw | example_content = '\n'.join([e + "\n" for e in example]) | 18:27 |
| blackboxsw | as all examples will now be an instance of string | 18:28 |
| blackboxsw | still good to avoid dependency if we don't really need it | 18:28 |
| smoser | yeah | 18:29 |
| powersj | blackboxsw: rharper thanks for comments, uploaded new version | 19:07 |
| smoser | powersj, of kvm ? | 19:09 |
| powersj | smoser: yes | 19:10 |
| powersj | smoser: thanks for the explanation re: arrays | 19:34 |
| blackboxsw | smoser: pushed your changes into https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/330243 + added the __doc__ fixes and an Examples SPACER for documentation where modules have > 1 example | 20:04 |
| blackboxsw | onto powersj branch | 20:05 |
| blackboxsw | powersj: have you pushed this to latest? https://code.launchpad.net/~powersj/cloud-init/+git/cloud-init/+merge/327646 | 20:08 |
| blackboxsw | you mentioned uploaded new version, but I don't see new commites | 20:08 |
| blackboxsw | you mentioned uploaded new version, but I don't see new commits | 20:08 |
| * blackboxsw tries a git fetch | 20:08 | |
| smoser | blackboxsw, he ammended / --force i think | 20:08 |
| smoser | i think | 20:08 |
| powersj | yeah trying to keep it down to a single commit | 20:09 |
| blackboxsw | +1 | 20:09 |
| blackboxsw | thx | 20:09 |
| powersj | sorry.... if you wanted to see the diff | 20:09 |
| blackboxsw | nah, just "broken" process for me when looking at a review I've commented on is seeing whether commits have come in afterward | 20:10 |
| blackboxsw | I need to figure out a better process for finding updates on branches | 20:10 |
| blackboxsw | how do you guys normally determine whether a branch you've reviewed has been updated? Just git fetch on the cmdline and watch whether a branch you care about updates? | 20:11 |
| blackboxsw | also, as you mentioned powersj, having actual commits of the diffs related to review comments speeds up the re-review because I can see what changed and if some comment was missed or interpreted a different way without hunting for the places where comments were made. | 20:13 |
| powersj | yeah... | 20:13 |
| blackboxsw | but, review 'speed' is not an issue when we leave a branch of yours floating in the breeze for weeks ;) | 20:13 |
| blackboxsw | https://www.youtube.com/watch?v=bcYppAs6ZdI | 20:14 |
| powersj | lol | 20:16 |
| blackboxsw | powersj: approved https://code.launchpad.net/~powersj/cloud-init/+git/cloud-init/+merge/327646 thanks man | 20:23 |
| powersj | blackboxsw: thx, if you do find a run that dies or has issues, please let me know :) | 20:23 |
| blackboxsw | powersj: will run it again right now | 20:24 |
| blackboxsw | what cmdline did you want? | 20:24 |
| blackboxsw | the test run I called or the ssh connect from paramiko that is failing | 20:24 |
| blackboxsw | 3 successes in a row on your latest branch | 20:32 |
| smoser | powersj, i'm ok with allowing image.execute to take a string or an array | 20:32 |
| smoser | and splitting it if it is a string | 20:33 |
| powersj | smoser: ok do you want me to go back and revert places where I changed arrays to strings? | 20:41 |
| powersj | blackboxsw: thanks for the re-test | 20:41 |
| smoser | powersj, just commented there. | 20:54 |
| smoser | wont your changes *break* lxd ? | 20:54 |
| smoser | ah. you changed that one to take a string | 20:54 |
| powersj | right and then res = self.pylxd_container.execute(['/bin/bash', '-c'] + [command] | 20:55 |
| powersj | tested both backends ;) don't want any regressions | 20:56 |
| smoser | http://paste.ubuntu.com/25492439/ | 21:05 |
| smoser | i'll propose that for merge really quick | 21:05 |
| smoser | powersj, https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/330459 | 21:09 |
| powersj | smoser: +1'ed | 21:11 |
| powersj | thank for that | 21:11 |
| powersj | thank you for that rather | 21:11 |
| blackboxsw | rharper: adapted util.json_loads to handle unserializable content, just pushed 95c1151..c000917 | 21:29 |
| blackboxsw | thanks for the review, https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/330115 is done | 21:29 |
| rharper | blackboxsw: nice! | 21:30 |
| rharper | and no instance-data.json is written. ? | 21:30 |
| rharper | still true? | 21:30 |
| rharper | in descr of commit | 21:30 |
| rharper | on MP | 21:30 |
| blackboxsw | ahh not true | 21:31 |
| rharper | IIUC, we won't throw TypeErrors anymore? | 21:34 |
| blackboxsw | lrharper: so, I'm pretty certain that no TypeErrors should be raised now during json_loads. But, I did leave the except TypeError: in get_data() just in case there is something I wasn't aware of. I didn't want some datasources to start raising TypeError exceptions on the json_loads call | 21:34 |
| blackboxsw | but generally the json.loads(default=our_default_handling_functor) should catch everything unknown to json | 21:35 |
| rharper | ok | 21:35 |
| blackboxsw | I'mchanging the commit msg for this to represent that | 21:35 |
| rharper | hrm, I'm still confused | 21:35 |
| rharper | we're parsing metadata ; then we want to dump the metadata which may have unserializble objects | 21:36 |
| rharper | we don't have an json to load before we dump; so we still have a path where the metadata might have something we cant serialize | 21:36 |
| rharper | no? | 21:36 |
| rharper | what would be needed is for json_dumps() to replace the unserializable ojbect with some replacement string indicating error or something like that ? | 21:37 |
| rharper | that may not be possible; however, we could pre-filter the metadata dictionary for content types that aren't json encodable (which is similar to the load IIUC) | 21:37 |
| blackboxsw | correct rharper, I added json_serialize_default() which returns the string 'Warning: redacted unserializable type {0}' for any unserializable keys/values | 21:39 |
| rharper | blackboxsw: I think you confused me with the json_loads; in dumps you set default=json_serialize_default, which looks like what I was suggested (replaces with string) | 21:39 |
| rharper | nice | 21:39 |
| blackboxsw | ahh sorry loads... my bad | 21:39 |
| rharper | sorry for the confusion | 21:39 |
| blackboxsw | I meant dumps. Sorry for using the wrong words ;) | 21:39 |
| rharper | excellent; that's really cool | 21:39 |
| blackboxsw | so again , I *think* that should handle all cases of unserializable content, but I left the try/except ValueError & log.warning message in place just in case we run into some other unforseen issue | 21:40 |
| blackboxsw | except TypeError rather | 21:41 |
| rharper | yep | 21:46 |
| smoser | powersj, i responded in that kvm merge | 21:50 |
| smoser | i have to run now... we can just order thes a bit different, and som elittle things i mentioned in line. | 21:50 |
| powersj | smoser: ok thanks | 21:52 |
| powersj | really appreciate it | 21:52 |
| blackboxsw | powersj: do you recall where #cloud-init irc logs are archived? I thought there was an IRC archive url somewhere which had historical #cloud-init logs | 22:01 |
| blackboxsw | I wanted to reference them in the meetingology meeting minutes from last meeting | 22:01 |
| powersj | blackboxsw: https://irclogs.ubuntu.com/2017/09/ | 22:02 |
| blackboxsw | that'd be it | 22:02 |
| blackboxsw | thanks | 22:02 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!