| niluje | blackboxsw: smoser: I just pushed the split of unittests | 10:22 |
|---|---|---|
| niluje | I hope this is okay | 10:23 |
| smoser | niluje, yeah. great. | 12:43 |
| smoser | in a MP its ok to add commits or to push --overwrite | 12:43 |
| smoser | adding commits is actually easier for the reviewer, and you can comment 'addressing review feedback' or something. | 12:44 |
| niluje | smoser: MP? | 12:48 |
| niluje | merge proposal? | 12:48 |
| smoser | yeah. | 12:49 |
| niluje | k | 12:49 |
| smoser | mp, pr, mr, pm | 12:50 |
| niluje | do you want me to merge the commits into one then? | 12:50 |
| smoser | all but the last oen make some sense :) | 12:50 |
| smoser | i'll squash it when i pull | 12:50 |
| niluje | okay | 12:50 |
| niluje | if you have anything you want me to change, feel free to ask :) | 12:53 |
| smoser | powersj, blackboxsw rharper comments on | 13:55 |
| smoser | https://public.etherpad-mozilla.org/p/qxREmZaciZ | 13:55 |
| smoser | are welcomed. | 13:56 |
| powersj | smoser: made one word change, otherwise looks good | 14:34 |
| smoser | hm.. | 14:35 |
| smoser | what did you change? i only see my color | 14:35 |
| smoser | blackboxsw, ^ i think you were working on a way to check that group... that we could use in c-i to inform people. | 14:37 |
| smoser | powersj, ^ | 14:37 |
| powersj | smoser: "If you have already signed the contributor agreement, please | 14:37 |
| powersj | simply say so here." removed the s on contributor | 14:37 |
| smoser | cool | 14:38 |
| smoser | thanks | 14:38 |
| smoser | blackboxsw, or rharper https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327525 . review appreciated. | 14:38 |
| blackboxsw | morning. | 15:01 |
| blackboxsw | will check it out | 15:01 |
| powersj | smoser: ho | 15:01 |
| rharper | smoser: stand up! | 15:04 |
| smoser | gah | 15:08 |
| smoser | rharper, blackboxsw https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327532 | 17:35 |
| smoser | blackboxsw, .... | 17:42 |
| smoser | wonder if it'd be helpful for your ipv6 stuff | 17:42 |
| smoser | if you grabbed the unit tests from https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/324274 | 17:42 |
| smoser | i was going to grab those into a separate merge propsoal, but realized it might make your life more difficult as you might make those tests fail | 17:43 |
| smoser | blackboxsw, well... i went ahead and grabbed. | 17:52 |
| smoser | https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327534 | 17:52 |
| blackboxsw | smoser: I think this ipv6 branch is going to be rather large as is. If you wanted to land that as a separate merge proposal I can just rebase against master to get the updates and fix the unit tests. I agree, this change will break those tests. | 17:53 |
| blackboxsw | yeah I'll review that. it's big enough as is. | 17:53 |
| smoser | blackboxsw, since i've got you.. | 17:54 |
| smoser | https://code.launchpad.net/~jcastets/cloud-init/+git/cloud-init/+merge/325740 | 17:54 |
| smoser | you asked for split up tests (or i asked you to ask for that) :) | 17:54 |
| smoser | i'm good with it as it is, but i'll add some comments on the privilged source port stuff. | 17:54 |
| smoser | i'm still kind of weary of | 17:54 |
| smoser | +from requests.packages.urllib3.connection import HTTPConnection | 17:54 |
| smoser | +from requests.packages.urllib3.poolmanager import PoolManager | 17:54 |
| blackboxsw | you think that needs to be from urllib3.poolmanager import PoolManager instead? | 18:01 |
| blackboxsw | or just don't like using PoolManager/HTTPConnection in general | 18:01 |
| smoser | blackboxsw, it seems that we're using an internal bit of requests | 18:28 |
| smoser | no ? | 18:28 |
| blackboxsw | smoser: I hadn't used requests yet in any projects I've touched. Yes, it feels a bit like that use here is diving into the repackaging of urllib3 within requests, which makes we wonder why not just use urllib3 directly in this case. The intent of the requests package seems was to make simple gets and posts or URIs a bit easier, but this invocation is diving into a more complex example which might be better | 18:33 |
| blackboxsw | pulled using urllib directly. | 18:33 |
| blackboxsw | or some other package | 18:34 |
| blackboxsw | or a higher level api within requests | 18:35 |
| * blackboxsw is reading up on the requests package | 18:35 | |
| smoser | blackboxsw, i dont have a good other solutionm, and it appears that this does work. | 18:38 |
| smoser | even on ubuntu, which does not have the 'vendorized' urllib3 | 18:38 |
| smoser | https://github.com/requests/requests/issues/4104 | 18:39 |
| smoser | and blackboxsw look at /usr/lib/python3/dist-packages/requests/packages/__init__.py on your system. | 18:40 |
| smoser | pints at https://github.com/kennethreitz/requests/pull/2375 | 18:40 |
| blackboxsw | now that is interesting issue. with the imports. Ok, so this looks like a known problem and different vendors handle things differently. So this might hit us with other non-ubuntu distros. | 19:05 |
| smoser | blackboxsw, tests/unittests/test_runs/test_simple_run.py | 19:59 |
| smoser | https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327344 | 19:59 |
| blackboxsw | tox -e | 20:04 |
| blackboxsw | oops | 20:04 |
| smoser | bah. no harlowja | 20:08 |
| smoser | he'd have an idea | 20:08 |
| smoser | blackboxsw, https://code.launchpad.net/~jcastets/cloud-init/+git/cloud-init/+merge/325740 | 20:18 |
| smoser | blackboxsw, so i figured it out... the source fo the 'lost mock' | 20:53 |
| smoser | get_cmdline() | 20:53 |
| smoser | sets a global PROC_CMDLINE based on the result, and once its called, it is then set forever. | 20:53 |
| smoser | not sure what the right thing to do here is | 20:54 |
| smoser | i guess really we should just need to mock that methods use | 20:55 |
| smoser | but *always* mock it | 20:55 |
| rharper | smoser: nice find | 21:02 |
| blackboxsw | thx smoser done testing https://code.launchpad.net/~jcastets/cloud-init/+git/cloud-init/+merge/325740. minor typo tweak on your comments | 21:02 |
| smoser | blackboxsw, thanks. | 21:04 |
| smoser | i'll pull | 21:04 |
| smoser | please think about that PROC_CMDLINE | 21:04 |
| blackboxsw | hrm, within unit tests couldn't we set DEBUG_PROC_CMDLINE then we'd never have the side-effect | 21:07 |
| blackboxsw | right smoser ? | 21:07 |
| blackboxsw | def get_cmdline(): | 21:07 |
| blackboxsw | if 'DEBUG_PROC_CMDLINE' in os.environ: | 21:07 |
| blackboxsw | return os.environ["DEBUG_PROC_CMDLINE"] | 21:07 |
| smoser | yeah... but how even to do that always ? | 21:08 |
| * blackboxsw was thinking about python-fixtures and the EnvironmentVariableFixture specifically in our unit test __init__ | 21:12 | |
| smoser | blackboxsw, i'm gonna kill you8r sscaleway | 21:15 |
| smoser | instnace | 21:15 |
| blackboxsw | smoser: thank you | 21:15 |
| smoser | blackboxsw, https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327344 is ready i think now. i'd appreciate a review | 21:33 |
| smoser | and /me goes away | 21:34 |
| blackboxsw | I need to head to the DMV to renew registration. relocating to wait in a line | 21:44 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!