| *** baoli has joined #openstack-sprint | 00:22 | |
| *** baoli has quit IRC | 00:27 | |
| *** baoli has joined #openstack-sprint | 04:32 | |
| *** baoli has quit IRC | 04:37 | |
| *** baoli has joined #openstack-sprint | 06:44 | |
| *** baoli has quit IRC | 06:50 | |
| *** yarkot1 has quit IRC | 08:11 | |
| *** yarkot1 has joined #openstack-sprint | 08:14 | |
| *** Zara has quit IRC | 09:37 | |
| *** Zara has joined #openstack-sprint | 09:38 | |
| *** baoli has joined #openstack-sprint | 11:06 | |
| *** cdelatte has joined #openstack-sprint | 11:07 | |
| *** baoli has quit IRC | 11:10 | |
| *** aarefiev has quit IRC | 12:05 | |
| *** electrofelix has joined #openstack-sprint | 12:55 | |
| *** rfolco has joined #openstack-sprint | 13:07 | |
| *** krotscheck is now known as lolscheck | 13:12 | |
| *** kien-ha has joined #openstack-sprint | 13:14 | |
| zxiiro | electrofelix: waynr I'm around | 13:16 |
|---|---|---|
| electrofelix | zxiiro: great, I'm just looking to add some tests for the fix to the bug you spotted with https://review.openstack.org/#/c/344742/1 | 13:20 |
| *** baoli has joined #openstack-sprint | 13:22 | |
| *** baoli has quit IRC | 13:27 | |
| *** lolscheck is now known as krotscheck | 13:31 | |
| electrofelix | zxiiro: I also spotted a small logging issue in https://review.openstack.org/319609, can fix it up now while sorting out the bug you spotted or by a patch on top of the series? | 13:36 |
| zxiiro | electrofelix: I'm fine with either. I think formatting is a minor issue | 13:39 |
| electrofelix | my thoughts as well, only reason I suggested it is I have to fix and rebase the entire series after that patch... | 13:42 |
| electrofelix | ok, due to start in about 10 minutes, grabbing coffee and be back shortly | 13:54 |
| electrofelix | waynr: you about? | 13:54 |
| *** hashar has joined #openstack-sprint | 13:57 | |
| *** baoli has joined #openstack-sprint | 13:57 | |
| waynr | just got online | 13:58 |
| waynr | this time i actually had breakfast before the sprint :) | 13:58 |
| hashar | I am floating around to show support :D | 14:03 |
| hashar | unlikely to be that helpful since I havent followed the JJB 2 effort :( | 14:03 |
| waynr | electrofelix: i thought about the discussion we had last time a little more regarding multiline strings vs multiple strings across each line and given that we can't use the multiline string in all cases other than docstrings I think it is reasonable for the sake of consistency to settle on not using them in argparser description strings | 14:04 |
| larainema | I am also would like to help, but I didn't follow much on JJB 2.0 :( | 14:06 |
| waynr | although i kind of want to file a bug against python for inconsistent treatment of multiline strings | 14:06 |
| waynr | larainema, hashar i don't think anyone except for myself was really familiar with the effort before the first JJB 2 sprint two weeks ago so now is a great time to start learning | 14:07 |
| waynr | i'll update the virtual sprint wiki page with some background reading for the JJB 2.0 effort but for now if you want to just review the patches just based on your python knowledge that would be helpful | 14:07 |
| waynr | here's the sprint wiki page: https://etherpad.openstack.org/p/jjb_api_v2.0 | 14:08 |
| electrofelix | waynr: it is annoying that there is no equivalent in code usage of multiline strings that works exactly the same as docstrings | 14:10 |
| waynr | electrofelix: what do you think about breaking down the TODO list into things that should happen on an existing patchset vs things that can be appended in a follow-on patchset? | 14:10 |
| electrofelix | I think all of them can be follow on? | 14:10 |
| waynr | okay cool | 14:11 |
| waynr | yeah what annoys me more than the inconsistency between multiline strings in code and multiline strings in docstrings is the inconsistency between various code usages of multiline strings (argparser vs logging) | 14:13 |
| waynr | although i might be misunderstanding the difference there | 14:14 |
| clarkb | docstrings are not syntactically special in python, you should be able to triple quote any string iirc | 14:16 |
| hashar | there is flake8-docstrings plugin to enforce format of docstrings :) | 14:16 |
| clarkb | maybe I dont understand the issue | 14:16 |
| electrofelix | clarkb: unfortunately you have to left align the string to avoid the extra spaces being retained and outputted, so either the formatting in the code looks bizare of the output format has lots of extra spaces and newlines unexpectedly | 14:18 |
| electrofelix | If you're doing it to a module global var, it works perfectly, since that can be aligned against the left most column are you're not going to see anything odd | 14:19 |
| electrofelix | moving the next line of string to the far left in the middle of code to avoid the extra spaces appearing in the output is a bit yarring to read | 14:19 |
| waynr | electrofelix: the behavior you are describing happens for logging and exceptions but not for argparser right? | 14:20 |
| electrofelix | yes, because argparser has a formatter built in that tidies it all up | 14:20 |
| electrofelix | you can use textwrap.dedent for exceptions and logging to get the formatting sensible that you're not going to have surprises grepping for messages and needing to deal with newlines | 14:21 |
| electrofelix | or extra spacing | 14:21 |
| clarkb | or use global constants or put up with the heredoc problem | 14:22 |
| electrofelix | true, but for logging messages and exception strings that doesn't seem to be the nicest approach should you wonder where the text came from | 14:22 |
| clarkb | honestly if its a ton of lines it really can be but that may be my C talking | 14:23 |
| clarkb | descriptive name often better than wall of text | 14:23 |
| electrofelix | Given that in most cases this was simply wrapping to a second line to avoid the 80 character limit, the resulting output was typically less than one line long | 14:24 |
| waynr | huh electrofelix there are a bunch of comments on July 8 on https://review.openstack.org/#/c/319611 that I didn't see the other night when looking for comments to address | 14:26 |
| electrofelix | I think I went back and marked them as fine today | 14:27 |
| electrofelix | waynr: ^^ | 14:27 |
| clarkb | electrofelix: thats not somehwere I would try to triple quote. I am clearly missing some context but for a log line wrap continuation single quotes with python's append behavior works great | 14:28 |
| waynr | okay I see now | 14:28 |
| waynr | clarkb: yeah that's what electrofelix recommended, my earlier comment in this channel was saying that we should also use that style for argparser for the sake of consistency even though argparser appropriately pretties up multiline strings | 14:29 |
| waynr | (last week i was hangry and arguing vehemently against line wrap continuation single quotes for argparser) | 14:29 |
| *** Kiall has joined #openstack-sprint | 14:30 | |
| waynr | i was trying to admit to being wrong ;) | 14:30 |
| electrofelix | clarkb: agreed, initially the different in output behaviour wasn't something that was realised in the patch series, and with argparser formatting it, unless you checked the log messages or exceptions fully you might think python multiline string worked exactly as simple concatenation | 14:30 |
| clarkb | gotcha | 14:30 |
| electrofelix | waynr zxiiro: looks like I've fixed up all the problems identified with https://review.openstack.org/#/c/344742 and subsequent patches are rebasing (going through tox of py35 of each one to confirm nothing got broken) | 14:32 |
| waynr | nice! looks like the other patches we wanted to review today don't yet have actionable comments | 14:33 |
| electrofelix | Aside from that patch all 5 other patches in the queue for today look like everyone is happy with them | 14:33 |
| electrofelix | excellent, we might be able to land them and a few others | 14:33 |
| waynr | aw yiss | 14:34 |
| electrofelix | I'll wait until you and zxiiro can review the fixes and the rework in the subsequent patch as it changes how they have to be handled. and then we can start on the next 2-3 patches | 14:34 |
| zxiiro | great | 14:36 |
| zxiiro | waiting for the update | 14:37 |
| electrofelix | do we want to go ahead and approve https://review.openstack.org/#/c/319609? | 14:37 |
| waynr | sure | 14:38 |
| hashar | that patch is scary and beautiful at the same time :} | 14:40 |
| electrofelix | hashar: the first one was even more scary :p | 14:41 |
| electrofelix | but I've gotten over my fear | 14:41 |
| waynr | fear is followed by acceptance | 14:43 |
| hashar | waynr: I should have taken time to look at those patches carefully. Overall that JJB2.0 spec and the patches are really a good idea | 14:45 |
| hashar | and I am surprised someone went bothering to do that large refactoring / cleanup. Kudos really | 14:46 |
| electrofelix | we will all owe waynr whatever he wants at the next summit | 14:46 |
| electrofelix | waynr zxiiro: one honking patch queue sent zuuls way ;) | 14:47 |
| waynr | :) thanks! it was a good learning opportunity for me also, not sure i'll be able to make the next summit sadly... | 14:49 |
| hashar | the Zuul status page shows a loooooot of jobs http://preview.tinyurl.com/gvs524t :} | 14:54 |
| hashar | assuming they are mostly merger requests due to the long JJB dependency chain | 14:54 |
| electrofelix | we're testing zuul's load handling capabilities ;) | 14:55 |
| waynr | ha | 14:57 |
| electrofelix | looks like there is a large cinder patch queue in there too | 14:58 |
| zxiiro | cool I'll take a look soon. I got a meeting right now that should be short. | 14:59 |
| electrofelix | grand, take it as we'll sync in about 30 minutes so | 14:59 |
| * waynr gets more caffeine | 15:19 | |
| zxiiro | electrofelix: I can confirm the updated patch fixes the authentication issue i was having (just realized i pasted in the wrong room earlier) | 15:19 |
| electrofelix | zxiiro: saw it in both places, just watching on the check queue | 15:20 |
| * waynr crawls out of the trying-to-figure-out-how-to-do-a-particular-thing-in-emacs rabbit hole | 15:26 | |
| waynr | i'm about to workflow +1 https://review.openstack.org/#/c/344742 unless anyone has any objections | 15:30 |
| electrofelix | :D | 15:32 |
| *** pleia2_ has joined #openstack-sprint | 15:32 | |
| *** pleia2 has quit IRC | 15:35 | |
| waynr | I'm gonna start picking off some of the TODOs since at this point I don't _think_ there is feedback to address on any of the unmerged patches | 15:39 |
| waynr | i'll mark the TODOs as I work on them as being WIP | 15:39 |
| *** morgabra has quit IRC | 15:39 | |
| *** vern has quit IRC | 15:39 | |
| *** krotscheck has quit IRC | 15:39 | |
| electrofelix | waynr: can you give some of the adjustments in https://review.openstack.org/319610 the once over to see if anything was missed? I've marked them with comments and also noted flush_cache getting dropped on the floor that can be fixed by a patch that doesn't have to be in the series | 15:41 |
| waynr | electrofelix: between patchset 6 and patchset 7? looking now | 15:42 |
| electrofelix | yeap, I think the flush_cache was an accidental omission from the original patch | 15:43 |
| *** krotscheck has joined #openstack-sprint | 15:47 | |
| *** morgabra has joined #openstack-sprint | 15:47 | |
| *** vern has joined #openstack-sprint | 15:47 | |
| *** pleia2_ is now known as pleia2 | 15:55 | |
| *** hashar has quit IRC | 16:00 | |
| electrofelix | waynr zxiiro: I think https://review.openstack.org/#/c/346102 should fix the flush_cache option handling without needing to rebase the entire series... I hope | 16:00 |
| electrofelix | I'm going to pick up the creation of static methods for creation using config object to allow for a very explicit __init__ method while keeping the default creation trivial, can then discuss whether that is better than just passing the config object into the init method once it's available | 16:05 |
| waynr | hmm electrofelix so looking at https://review.openstack.org/319610/7 it looks like it now has some of the changes that were originally introduced in https://review.openstack.org/#/c/319611 | 16:07 |
| waynr | (this changed between patchset 6 and patchset 7) | 16:07 |
| waynr | looks like https://review.openstack.org/#/c/319611 is now in mergeconflict state | 16:08 |
| electrofelix | nuts, I must have done an amend by accident | 16:10 |
| electrofelix | during the conflict resolution | 16:11 |
| waynr | it'll be easier to see the adjustments you mentioned earlier once that's fixed | 16:11 |
| zxiiro | sorry got pulled in another meeting | 16:32 |
| zxiiro | where are we at? | 16:32 |
| electrofelix | I screwed up the rebase | 16:32 |
| zxiiro | ETA for fix? | 16:33 |
| electrofelix | approx 5 mins, I'll push up the first 4 in the series while checking the remainder with tox locally | 16:34 |
| electrofelix | waynr: https://review.openstack.org/#/c/319611/6..7 and https://review.openstack.org/#/c/319610/6..8/jenkins_jobs/cli/entry.py look a bit more sane now to me for what differences there should have only been included | 16:36 |
| *** morgabra has quit IRC | 16:38 | |
| *** morgabra has joined #openstack-sprint | 16:38 | |
| electrofelix | waynr: I could be wrong, but I think the only differences showing up in https://review.openstack.org/#/c/319610/6..8/jenkins_jobs/cli/entry.py are from the fix to the earlier patch for handling options from the CLI overriding the default config incorrectly, hence gerrit applied the automatically +2 from myself and zxiiro on revision 6 to the latest patch automatically | 16:40 |
| zxiiro | electrofelix: seems like it. it probably picked it up as a rebase or something | 16:43 |
| waynr | electrofelix: okay that looks better, i just compared the revisions 3..8 and it looks sane to me considering the new change you added that we just merged today | 16:44 |
| electrofelix | ok, going to approve https://review.openstack.org/#/c/319610 barring any objections? | 16:47 |
| electrofelix | https://review.openstack.org/#/c/319611 will need more reviews, but once that is acceptable I'll approve both it and https://review.openstack.org/#/c/319612 | 16:47 |
| waynr | okay, looking now | 16:48 |
| electrofelix | After that it's https://review.openstack.org/#/c/319613/8 to be reviewed, and I think that puts us ahead of originally planned :) | 16:48 |
| waynr | hmm it's getting harder to see the differences between patchsets | 16:49 |
| waynr | probably https://review.openstack.org/#/c/344742 would have been better as a follow-on change (just making a note for future big batches of work like this) | 16:50 |
| electrofelix | having gone through it, I wish I had done it that way in the first place... | 16:51 |
| zxiiro | sweet 3 more patches | 16:55 |
| waynr | https://review.openstack.org/#/c/319611 looks ready for merge to me | 16:56 |
| electrofelix | trigger pulled | 16:56 |
| waynr | pew pew | 17:02 |
| electrofelix | I think we'll have to wait for some of the CI jobs to come back, but once they do, are we happy to approve the following set of changes? | 17:10 |
| electrofelix | https://review.openstack.org/#/c/346102 - Ensure flush cache option obeyed | 17:10 |
| electrofelix | https://review.openstack.org/319612 - Use JJBConfig in ModuleRegistry | 17:11 |
| electrofelix | https://review.openstack.org/319613 - Move load_files method to YamlParser from Builder | 17:11 |
| electrofelix | https://review.openstack.org/#/c/319614 - Simplify 'update and delete old jobs' test (needs another +2) | 17:11 |
| electrofelix | last one is relatively straight forward as well | 17:11 |
| zxiiro | +1 i think they are all good to go | 17:12 |
| electrofelix | That would bring the list down to 20 remaining patches, and I think one more sync up would probably cover enough of those that the rest will start to land automatically. | 17:12 |
| electrofelix | Anyone have any questions about the V2 api patches in general? | 17:13 |
| zxiiro | nope, I do want to try to land the views patch at some point, maybe even the folders patch too. | 17:13 |
| electrofelix | definitely | 17:14 |
| zxiiro | we maintain a lot of views in our project so being able to manage them with jjb would be incredibly handy since we're currently doing it by hand | 17:14 |
| electrofelix | I think one more sync up in general and then a hopefully a follow up to tie up loose ends and plan next pieces that should be | 17:15 |
| electrofelix | worked | 17:15 |
| zxiiro | sounds good | 17:15 |
| waynr | sounds good to me, i've blocked off this four hour time slot every two weeks for the next two months just in case, sounds like we feel pretty confident that should cover most of the V2 patches | 17:16 |
| waynr | I also just submitted https://review.openstack.org/#/c/346136/ by the way | 17:17 |
| waynr | (Move JJBConfigException to errors module) | 17:17 |
| electrofelix | just +2'ed it since it's a straight forward move, wait a little while before approving to let all the check jobs get back | 17:19 |
| electrofelix | Looks like they should start finishing in about 15 minutes, so we can start approving them then | 17:20 |
| electrofelix | otherwise, great to see progress and enjoy the weekend | 17:20 |
| waynr | electrofelix++ thanks again for arranging these sprints! | 17:22 |
| waynr | (and for enthusiastically making the changes you want to see to the API work) | 17:23 |
| zxiiro | +1 thanks for all the effort | 17:29 |
| electrofelix | I've approved the next three as well | 17:52 |
| zxiiro | yay, thanks everyone, great progress today :) | 17:53 |
| electrofelix | Added same time slot for 5th and 19th of August | 17:59 |
| *** mrmartin has joined #openstack-sprint | 18:01 | |
| electrofelix | final changes approved have merged, in all 8 patches landed on the march towards V2.0 nirvana | 18:11 |
| electrofelix | 20 + some additional adds to go | 18:11 |
| electrofelix | Going to go ahead and approve the other two minor fixes | 18:13 |
| electrofelix | https://review.openstack.org/#/c/346136/ - Move JJBConfigException to errors module. | 18:13 |
| electrofelix | https://review.openstack.org/346102 - Ensure flush cache option obeyed | 18:13 |
| electrofelix | especially the flush cache one in case someone is using the CLI with the latest head | 18:13 |
| electrofelix | I'll leave the etherpad as is for now, and tidy it up next week | 18:15 |
| electrofelix | enjoy the weekend folks | 18:15 |
| *** electrofelix has quit IRC | 18:17 | |
| *** kien-ha has quit IRC | 18:36 | |
| *** mrmartin has quit IRC | 19:12 | |
| *** mrmartin has joined #openstack-sprint | 19:12 | |
| *** hashar has joined #openstack-sprint | 19:40 | |
| *** mrmartin has quit IRC | 20:56 | |
| *** rfolco has quit IRC | 21:07 | |
| *** cdelatte has quit IRC | 21:08 | |
| *** hashar has quit IRC | 21:29 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!