Wednesday, 2017-02-15

*** VW has joined #craton00:03
jimbakerthomasem, i can combine00:06
jimbakerlet's just do that00:06
jimbakerso i'm doing a final review of sigmavirus' pagination. there are a couple of issues that thomasem raised, but since this is touching a lot of code, i want to get this in, and go back fix the minors00:06
jimbakerthe other issue is that the validation does not allow sort_keys and sort_dir to be specified, but again this is an easy fix00:07
jimbaker(at least for /v1/hosts)00:07
*** VW has quit IRC00:08
jimbakerthomasem, ok, i hope no pitchforks for my approving https://review.openstack.org/#/c/42622900:46
jimbakerbut it seemed the right thing to do imho00:46
jimbakergiven no one is actually yet consuming those links00:47
jimbakerof course one implication is that client/CLI is no longer working, because it expects a different response body. this will be a trivial fix. just FYI01:01
jimbakermove fast, break things???01:01
jimbakerback to curl for the moment :)01:01
*** Syed__ has quit IRC02:45
*** serverascode_ has joined #craton03:05
*** keekz_ has joined #craton03:06
*** _d34dh0r53_ has joined #craton03:06
*** keekz has quit IRC03:07
*** serverascode has quit IRC03:07
*** d34dh0r53 has quit IRC03:07
*** keekz_ is now known as keekz03:07
*** serverascode_ is now known as serverascode03:18
*** tojuvone has joined #craton07:26
*** mdorman has quit IRC11:53
*** mdorman has joined #craton11:55
sigmavirusjimbaker: the bug you filed for the client is a dupe13:11
sigmavirusthere's a separate project for client bugs13:11
sigmavirusjimbaker: also, are you planningon picking up the clientn work I had started or??13:26
*** VW has joined #craton13:56
jimbakersigmavirus, sorry, i forgot we tracked distinctly when i filed, doh. thanks for fixing that dupe14:04
sigmavirusjimbaker: no worries :)14:04
jimbakersigmavirus, i do have an outstanding fix for the client/CLI in https://review.openstack.org/#/c/434068/14:04
jimbakerit's modest14:04
thomasemjimbaker: Honestly, the only pitchfork you're going to see from me is how there's expediency around pagination which isn't on the priority list we've gone over several times now (unless that's changed), while my project vars patch finds itself in yet another merge conflict.14:04
jimbakerand maybe there is a better way of doing things14:05
jimbakerthomasem, i suppose i believe everyone gets to enjoy some merge conflicts14:05
thomasemOf course. That's the nature of it. I'm only concerned about the priorities and this being delayed further as a result.14:06
sigmavirusjimbaker: that doesn't handle pagination for the users :) My patch was focusing on that14:06
jimbakeri understand that pagination had lower priority. but it does work14:06
sigmavirusthomasem: you're right14:06
sigmavirusthomasem: do you want help with the merge-conflict?14:06
sigmavirusI am ... kind of experienced now at handling them quickly14:07
thomasemNah, it's cool. I'm digging in right now.14:07
thomasemLol, same. :)14:07
thomasemThe only reason I say anything is because we set these priorities and then continue to get distracted from them. I appreciate that pagination has been in flight for a while and having merge conflicts and such the same as everyone else.14:07
jimbakeri assume that the minors identified by thomasem in sigmavirus' patch were all because of merge conflict fixes14:08
jimbakerthomasem, the other thing we got in the deal is the links support14:08
thomasemI get that, and do appreciate it. I will shut up now.14:09
jimbakermerge conflict fixes/not quite merge conflict fixes14:09
thomasemOh, yeah. Maybe they were?14:10
thomasemWas that regressed code to an earlier state or something?14:10
jimbakerthomasem, they have the classical presentation of a merge regression14:10
thomasemAnyway, the minors I identified do not actually cause a bug or anything. It was just extra code.14:10
jimbakerindeed. tiny amount of tech debt compared to progress attained. ruthless i am14:11
jimbaker;)14:11
thomasemYeah14:11
jimbakerso i would like some fix for the client/CLI in ASAP because right now it's BROKEN. i have a proposed solution, sigmavirus has some other work. whichever it is, let's get it fixed14:13
sigmavirusthomasem: to be fair, I really wasn't expecting the pagination work to get merged14:18
thomasemIt's all good. I probably had a really poor tone coming across. I would pretty much always add a smirk at the end of everything I say. :) My main motivation is keeping us accountable to the goals we set for the next few weeks here. :)14:19
thomasemUltimately, I'm glad it did. It is a very useful thing to have.14:20
sigmavirusthomasem: no don't worry about your tone14:20
sigmavirusI've been saying the same things since before you got here14:21
thomasemLol, ahhhh, gotcha.14:22
sigmavirus"to be fair" is my way of saying "While I'm relieved it finally merged"14:24
sigmavirus(I really should just have said that)14:24
thomasemLOL14:26
thomasemThat's perfectly fair14:26
thomasemsigmavirus: question! https://review.openstack.org/#/c/426229/11/craton/db/sqlalchemy/api.py line 494, why'd you remove _paginate wrapping the return? Looks like that broke a test I added in my patch.14:27
thomasemSince it seems projects_get_by_name wasn't tested before14:28
sigmavirushm14:28
thomasemIt's expecting the links to be returned as well as the query result14:28
sigmavirusThat looks like git did something "helpful" when resolving a conflict14:28
thomasemBummer14:28
sigmavirushttps://i.imgur.com/26k9Td7.gif14:29
thomasemLol14:30
thomasemAlright, cool. I'll just add back what was there.14:30
jimbakereveryone gets something with a breaking change :)14:30
thomasemWell, fortunately this wont' happen again, because tests!14:30
thomasemwon't*14:30
jimbakerwe hope. the tests are getting better...14:31
thomasempatch-by-patch14:31
thomasem'tis all you can do14:31
*** jovon has joined #craton14:34
sigmavirusthomasem: added a change for that14:43
sigmavirusworking on tests14:43
thomasemsigmavirus: oh, I was fixing it in my patch already...14:44
sigmavirusI need to turn notifications on for this channel (I think we can do it now)14:45
thomasemLol14:45
sigmavirus(Gerrit -> IRC notifications)14:45
sigmavirusI started hacking on it when you pointed it out14:45
thomasemI have a test that exercises it. Though something's up with my environment and old code is executing, so about to blow away my clone.14:45
thomasemsigmavirus: ^^ and, yep, notifications would be nice14:47
thomasemWhat on earth? It's not executing my craton/db/sqlalchemy/api.py in the tests...14:50
thomasemI blew away the entire directory and it's still not14:50
thomasemOhhhhhhhhhhhhhhhh14:53
thomasemI'm a baddie. That's why.14:53
sigmavirusoh?14:54
sigmavirusI'm adding a functional test at the moment because that's slightly easier14:54
thomasemDidn't fix my mock up properly.14:54
thomasemNothing to see here...14:54
thomasemI'm ashamed now.14:54
* sigmavirus thinks we use mock too much in our db tests14:54
* sigmavirus is uncool with the popular crowd14:55
sigmaviruswhile my functional tests run, I'll brb14:55
tojuvoneEvening Craton & friends15:03
thomasemAlright, fixed up and running functional tests also.15:10
thomasemhey tojuvone!15:10
jimbakertojuvone, welcome back. great vacation in london?15:11
tojuvonejimbaker, Thanks, as my son said, best trip ever :)15:12
tojuvonebut very cold15:12
jimbakerno doubt, a harry potter fan15:12
tojuvonebtw, I put some thought here: https://tomijuvonen.blog/2017/02/10/planned-host-maintenance/15:14
tojuvoneAlso there is a mail from Curtis in Openstack-operators mailing list. Thought to respond.15:19
sigmavirusthomasem: so if you want to throw your db test onto my change, that would be good15:20
sigmavirusHi tojuvone!15:20
sigmavirushttps://review.openstack.org/43431915:20
*** Syed__ has joined #craton15:24
thomasemsigmavirus: done and pushed15:31
sigmavirus:cake:15:31
sigmavirusI use GitHub emoji as if they work everywhere =P15:31
thomasem:wookieedance:15:31
sigmavirusWait, is that a GitHub Emoji?15:32
thomasemI use custom Slack emoji's as if they work everywhere.15:32
sigmavirusah15:32
sigmavirushaha15:32
thomasemIt ought to be, imo.15:32
thomasemWhy it isn't yet is so far beyond me.15:32
thomasemjimbaker: can we get a review of this https://review.openstack.org/#/c/434319?15:33
jimbakerthomasem, sure15:33
thomasemquick change since it's broken now15:33
jimbakerwe are going to get that in before the project vars, which i'm reviewing now and about to -115:33
jimbakersince i found a bit of brokeness in it15:33
thomasemThat's fine. It's dependent anyway now.15:34
jimbakerlet me finish that review first, then i will review sigmavirus' (and yours)15:34
jimbakercool15:34
jimbakerthomasem, reviewed project vars. do take a look at the logger as well15:38
jimbakerbut you should have everything you need to reproduce from my review15:38
thomasemWhat on earth15:39
thomasemThanks jimbaker, looking into it. Something went sideways15:39
jimbakerpossibly a merge problem15:39
thomasemWouldn't be surprised15:39
jimbakerwhatever it is, should be easy to fix15:39
jimbakeri'm more concerned about the logger setup15:40
jimbakeror at least equally concerned15:40
jimbakerincidentally this is a good example of where we need to do a better job of producing json15:40
jimbakeri am now in a habit of always doing this15:41
jimbaker$ curl http://127.0.0.1:8080/v1/projects/b9f10eca-66ac-4c27-9c13-9d01e65f96b4/variables -H "Content-Type: application/json" -H "X-Auth-Token: demo" -H "X-Auth-User: demo" -H "X-Auth-Project: b9f10eca66ac4c279c139d01e65f96b4" | jq # --- but this use of jq fails because it just gets "A server error occurred.  Please contact the administrator."15:42
jimbakerin other places we do return a JSON body in case of error15:42
thomasemThe API WG guidelines say to return JSON errors, iirc.15:42
jimbakerthomasem, yep. so spit & polish :)15:43
jimbakershould be easy enough to ensure, much like with returning indent=2 and sort_keys=True for the json response15:44
jimbakerwill file a bug15:45
thomasemWhat you found is something to do with the `demo` user15:45
thomasemusing bootstrap it works just fine15:45
jimbakerthomasem, but the demo user should be created by the bootstrap process so it just works, right?15:48
jimbakermy assumption is that bootstrap is just there to get us to a REST API createable projects/users15:48
thomasemI meant the bootstrap user.15:49
thomasemthe bootstrap user works fine, demo does not.15:49
thomasemjimbaker: ^^15:49
thomasem$ curl http://127.0.0.1:8080/v1/projects/b9f10eca-66ac-4c27-9c13-9d01e65f96b4/variables -H "Content-Type: application/json" -H "X-Auth-Token: demo" -H "X-Auth-User: demo" -H "X-Auth-Project: b9f10eca66ac4c279c139d01e65f96b4"15:49
thomasemA server error occurred.  Please contact the administrator.15:49
thomasem$ curl http://127.0.0.1:8080/v1/projects/b9f10eca-66ac-4c27-9c13-9d01e65f96b4/variables -H "Content-Type: application/json" -H "X-Auth-Token: bootstrap" -H "X-Auth-User: bootstrap" -H "X-Auth-Project: b9f10eca66ac4c279c139d01e65f96b4"15:50
thomasem{"variables": {}}15:50
jimbakerright. so we should make sure the demo user does work as expected15:50
thomasemwhich makes precisely 0 sense to me.15:50
jimbakeri don't expect the bootstrap user to work at all :)15:50
thomasemI get that. I'm more perplexed by how that even HAS an impact here.15:50
thomasemSome weird side effect15:50
jimbakeryep. so anyway... code15:51
jimbaker;)15:51
thomasem?15:51
sigmavirusheh15:51
jimbakercode can be weird15:51
jimbakeruntil we figure it out15:51
thomasemNaturally15:51
thomasemAnd then it can still be weird because it's implemented poorly! :D15:51
jimbakerhopefully our layers are not so complex here. it would not seem to be the case15:51
jimbaker:)15:51
jimbakerbut not so here i think15:51
thomasemMmmkay15:52
jimbakerthomasem, ok, hopefully this small starter "project" i assigned you will complete at some point15:52
jimbakerclearly we have realized it was a little more than that15:52
jimbakerand yes, i think it is close15:53
jimbakerto being done15:53
thomasemYeah, that's no problem. Lots of problems are icebergs.15:53
jimbakerindeed15:53
thomasemSo, non-admin is causing oslo_db to try to check for 'project_id' in the record you're accessing for generic multi-tenancy...16:04
thomasemAlright, so this is even more complicated, then16:06
thomasemjimbaker: This is due to the generic variable stuff necessitating removing the admin check for this function16:07
jimbakerthomasem, yep, that would make sense16:07
jimbakerhttps://bugs.launchpad.net/craton/+bug/166501516:07
openstackLaunchpad bug 1665015 in craton "All text errors reported by the Craton REST API should be JSON encoded" [High,New]16:07
thomasemSo, now when you try to access the project as non-admin (because the generic variable stuff doesn't support admin vs. non-admin distinction) oslo_db blows up16:07
*** VW_ has joined #craton16:09
thomasemThinking about how we want to support that (or work around it), then.16:09
*** VW_ has quit IRC16:09
jimbakerthomasem, so what should we do then? i'm going to suggest that we get this current project vars support in now, and then fix in a subsequent patch16:10
*** VW_ has joined #craton16:10
thomasemjimbaker: well, we could give demo user admin temporarily16:10
thomasemI am hoping the RBAC solution will solve the rest... otherwise we need to shim something into the generic variables stuff to respect admin/root.16:11
jimbakerthomasem, we will want to do the shim first. rbac is post cmdb milestone16:11
thomasemYeah16:11
thomasemJust trying to conceive what that might look like.16:11
jimbakershim should be reasonable. just need to add the necessary context stuff in the generic variables support16:12
thomasemThinking a list of resources mapped to their permissions16:12
*** VW has quit IRC16:12
jimbakermini-rbac16:12
thomasemBasically16:12
thomasemNot my favorite, but until we have proper RBAC, it'll have to do.16:13
jimbakerso just to be clear, we already have all the necessary info to do mini-rbac16:13
jimbakerin the schema16:13
thomasemYes, it's a matter of leveraging it.16:13
jimbakerit just needs to be used by that generic vars support. so yeah, let's just separate and get your work in16:13
jimbakerok, i'm going to +2 now unless we can think of an objection16:14
jimbaker(or we can wait 30 min...)16:14
thomasemjimbaker: If you want to merge https://review.openstack.org/#/c/434319/ first, I'll clean up my patch and push a new one up. Then we can merge that one and I'll follow up with some sort of mini-rbac thing to fix the oslo_db issue.16:17
thomasemThat can all happen within 30 min16:18
thomasemI also have the LOG fix in, so16:19
thomasemthat logs properly now and sends a JSON error16:19
thomasemOr we could merge my project vars patch and I'll just clean up sigmavirus's patch to just be the functional test addition.16:24
thomasemIt's either/or, really16:24
sigmavirusthomasem: you can rebase your patch on top of mine16:26
sigmavirusand gerrit will understand16:26
* sigmavirus isn't sure how familiar you are with gerrit/git-review16:26
thomasemI'm getting there. It's been a long time since I've used it. :)16:28
*** VW_ has quit IRC16:30
*** VW has joined #craton16:30
sigmavirusso you can git review -d <Change-ID> to download a patch and then git review -x <Other-Change-Id> to have git-review rebase it atop that16:30
sigmavirus(since the branches git-review makes are usually terrible and I hate using my mouse to copy-paste)16:30
sigmavirus(yes I'm that lazy)16:31
thomasemNice trick. Thank you.16:32
jimbakersigmavirus, thanks for the beta16:38
jimbaker(using the climbing term)16:39
* sigmavirus doesn't climb16:39
jimbakeranyway, 5 min in vidyo. quick dog walk16:39
sigmaviruswhat about vidyo?16:39
faridsigmavirus: you should have an invite, do you not ?16:42
faridguess not16:43
faridyou do now :)16:43
sigmavirushah16:44
sigmavirusthomasem: after destroying some dangling (cached) images, i can reproduce the functional failures16:51
sigmavirusAre you already tackling those or should I go for it?16:52
thomasemAhhhh, gotcha16:52
thomasemYeah, it's a permissions problem. I'll tackle it. I already addressed it in my patch for my other functional tests.16:52
thomasemMatter of doing the same thing here.16:52
sigmavirusGot it16:52
sigmavirusOh I should go add a functional test gate to our project16:52
thomasem+10016:52
sigmavirushttps://review.openstack.org/434391 < Gerrit -> IRC notifications17:06
sigmavirushttps://review.openstack.org/434399 < Functional testing17:06
jimbaker+100017:09
jimbakeri take your +100, and raise it exponentially :)17:10
jimbakerbut it is an annoying aspect of the review process to have to do tox -e functional. i rather focus on kicking the tires17:11
jimbaker(if indeed that's what people do. because kicking tires sounds painful)17:11
sigmavirusjimbaker: only kick tires when wearing steel-toe boots17:15
jimbakersigmavirus, got it. sounds like a new test tool name17:17
sigmaviruslol17:18
jimbakerspeaking of irc notifications, https://review.openstack.org/434399 needs a minor fix17:22
jimbakersigmavirus, ^^^17:23
jimbakerand given it's been raised to +1000 as something we could benefit from :), just mentioning it17:24
sigmavirusthanks jimbaker17:24
sigmavirusjimbaker: should be fixed up now17:25
thomasemsigmavirus: part of the problem with that functional test is how it requires the bootstrap user to be set up and all sorts of other stuff that my patch introduces... I'm actually wondering if we wouldn't be better off squashing these, if we're going to gate on functional tests for that too.17:29
sigmavirusthomasem: the three patches?17:33
* sigmavirus shrugs works for me17:33
thomasemALL THE PATCHES17:34
thomasemSquash them all. Mega review.17:34
sigmavirusthomasem: I've seen that before :(17:34
thomasemOh dear17:34
thomasemI'm sorry17:34
sigmavirusIt's not your fault.17:35
Syed__sigmavirus: i didn't knew the way to rebase your change on top of another change via git review -x17:35
sigmavirusSyed__: stick with me, I know way too many oddly specific tricks17:35
Syed__sigmavirus: :)17:35
jimbakergit-harry, https://bugs.launchpad.net/craton/+bug/1665050 - now we have captured it, and can work when get there17:37
openstackLaunchpad bug 1665050 in craton "Support alternative variable overrides" [Undecided,New]17:37
jimbakerit is explicitly NOT in scope for the cmdb milestone17:37
jimbakereven though it was brought up in our discussions, specifically with respect to cruton17:37
thomasemCome on functional tests, please pass this time.17:37
jimbakerthe good thing is, we probably only get to suffer through this very much now and then. i hope.17:39
thomasemI hope as well.17:39
thomasemSurely this is shortening my life span.17:39
thomasem:P17:39
Syed__haah17:39
jimbakereveryone, take deep breaths...17:40
jimbakerthomasem, so i assume you are fixing up https://review.openstack.org/#/c/434319/ ?17:46
jimbaker(as in, functional testing failing)17:47
thomasemjimbaker: yes, but I'm having to in my patch. I will just squash these, if that's okay, and we'll merge the change over here https://review.openstack.org/#/c/42777717:50
jimbakeryes, let's just do that17:50
thomasemReason being - for those functional tests to work, it has to rely on the bootstrap stuff I added.17:50
jimbakerexactly17:50
thomasemExcellent17:50
*** valw has joined #craton17:51
* sigmavirus lunch17:55
*** VW has quit IRC18:12
*** VW has joined #craton18:12
*** VW has quit IRC18:13
*** VW has joined #craton18:13
thomasemI need to lunch as well, I'll have to finish this when I get back.18:18
jimbakeranother one to add to our in-scope work, but also easy (really, i know it is, i just to pull it out of a migration i did for my RBAC prototyping)18:19
jimbakerhttps://bugs.launchpad.net/craton/+bug/166506618:19
openstackLaunchpad bug 1665066 in craton "Alembic script must use named constraints to support future upgrades" [Critical,New] - Assigned to Jim Baker (jimbaker)18:19
*** VW_ has joined #craton18:30
*** VW_ has quit IRC18:31
*** VW_ has joined #craton18:31
*** valw has quit IRC18:32
*** VW has quit IRC18:33
faridjimbaker and team - cloudnull was very helpful in pointing me to what he's been using in his tests, it's https://github.com/cloudnull/cruton/tree/master/playbooks18:34
faridcurrently getting some test data together though18:34
*** valw has joined #craton18:35
jimbakerfarid, awesome18:37
*** VW_ has quit IRC18:37
jimbakerand thanks cloudnull18:37
*** VW has joined #craton18:38
faridjimbaker: got this for now https://gist.github.com/anonymous/0885ce1944523e0b78d898457b17f9e318:39
faridthink it should be a good start18:40
jimbakerfarid, very much so. we should add a link to that data in our cmdb etherpad18:42
faridyep will do18:45
*** jovon has quit IRC18:52
mudpuppy:)19:02
*** valw has quit IRC19:03
sigmavirusthanks mudpuppy :)19:08
*** VW has quit IRC19:22
*** VW has joined #craton19:29
*** valw has joined #craton19:31
*** valw has quit IRC19:35
*** valw has joined #craton19:51
sigmavirushttps://review.openstack.org/#/c/434399 is passing now20:02
sigmavirusI'm just testing https://review.openstack.org/#/c/429040/ on my dev env20:02
sigmavirusjimbaker: thomasem y'all around?20:19
jimbakersigmavirus, so we can do https://review.openstack.org/#/c/429040/ instead of my patch?20:27
sigmavirusyep, it does more and adds tests20:28
sigmavirusmy problem testing it is that our json-schema doesn't allow sort-dir/sort-keys20:28
sigmavirusso20:28
sigmavirusI'm going to go ahead and change the schema20:28
sigmavirusJust wanted to make sure no one had serious objections to that20:28
jimbakersigmavirus, please make those changes to the schema20:29
jimbakerthat's the essence of my bug report in https://bugs.launchpad.net/craton/+bug/166478620:30
openstackLaunchpad bug 1664786 in craton "Fix minor problems identified in recent pagination work" [Critical,New] - Assigned to Ian Cordasco (icordasc)20:30
jimbakerspecifically in my comments about the schema in https://review.openstack.org/#/c/426229/1120:30
jimbakerotherwise, pagination links don't work, etc20:30
jimbakersigmavirus, and in the meantime, let me review that client work; and i will abandon my patch20:31
jimbakerany reason why https://review.openstack.org/#/c/434444/ hasn't merged yet?20:32
jimbakerand https://review.openstack.org/#/c/434433/?20:32
sigmavirusit's not on 43444420:33
sigmavirushttp://status.openstack.org/zuul/20:33
sigmaviruslet's see if adding a new vote to it updates it20:34
Syed__yeah thats what i checked20:34
Syed__it doesn't show it to be at zuul20:34
Syed__Same with 43443320:34
sigmavirusHonestly I think zuul is struggling right now for some reason20:35
sigmavirusI saw a lot of "POST_FAILURE" results recently20:35
jimbakersigmavirus, got it20:37
sigmavirusah looks like logs.openstack.org is the struggler20:37
sigmaviruscan't delete old files fast enough to keep up with the upload of new log files20:37
jimbakersigmavirus, i like the generator in the client, very nice20:38
thomasemsigmavirus: I'm around now20:38
sigmavirusIt also required no updates to work with print_list which was nice20:38
jimbakerjust need to get the schema change in, but clearly it's generating the correct links before the api barfs20:39
jimbakerit = client20:39
sigmavirusyep20:40
sigmavirustesting the schema change locally first20:40
sigmavirus=P20:40
jimbakernot a problem20:42
sigmavirusI pushed it for review, but want to quickly check it20:43
*** VW has quit IRC20:49
git-harryjimbaker: it depends on https://review.openstack.org/#/c/434342/120:52
jimbakersigmavirus, you are going to want to revisit the schema fix here; my local testing is getting a 500 due to JSON formatting; GET /v1/hosts?marker=10&region_id=1&limit=30&sort_dir=asc&sort_keys=created_at%2Cid20:52
sigmavirushm20:52
jimbakersigmavirus, are you sure you don't want to generate this as &sort_keys=created_at&sort_keys=id20:52
jimbakeri'm pretty sure that works just fine20:53
* sigmavirus wonders what is causing that 50020:53
sigmavirusoh20:53
sigmavirusI suspect we need some pre-processing on the keys20:53
sigmavirusI think the preferred manner is wht I have20:53
jimbakeryeah, because of the validation20:54
sigmavirus*API-WG manner20:54
jimbakeri'm good if we can flask-restful accept20:54
jimbakercan get20:54
sigmavirusthat said, my testing shows host-list working fine20:54
sigmavirushm20:54
sigmavirusweird20:55
sigmavirusoh20:55
sigmaviruslet me try adding sort keys20:55
-openstackstatus- NOTICE: We're currently battling an increase in log volume which isn't leaving sufficient space for new jobs to upload logs and results in POST_FAILURE in those cases; recheck if necessary but keep spurious rebasing and rechecking to a minimum until we're in the clear.20:55
sigmavirus^ jimbaker20:55
jimbakergit-harry, ahh, ok, i will take a look20:55
jimbakeranother important change20:56
*** VW has joined #craton20:58
sigmavirusjimbaker: updated with some processing on sort_keys20:58
jimbakersigmavirus, awesome21:00
sigmavirushaving uninterrupted time is surprisingly good for productivitty21:02
* sigmavirus is heading out21:03
sigmaviruslater yinz21:03
jimbakersigmavirus, ok if i update https://review.openstack.org/#/c/434506/ given conflicts with git-harry's recent work?21:04
sigmavirusgopher it21:05
* jimbaker wants everything working end-to-end again for some reason :)21:05
jimbakerthanks!21:05
jimbakersigmavirus, i'm still getting this: TypeError: InvalidSortKey('Sort key supplied is invalid: created_at,id',) is not JSON serializable21:06
jimbaker127.0.0.1 - - [15/Feb/2017 14:05:56] "GET /v1/hosts?marker=10&limit=30&sort_dir=asc&region_id=1&sort_keys=created_at%2Cid HTTP/1.1" 500 5921:06
jimbakerok, we will just wait a bit longer21:06
jimbakeri'm going to get a run in. as usual, late21:06
jimbakerfwiw, i test this by going into the client, and doing this: list(craton.hosts.list(region_id=1))21:08
jimbakerso that generator works fine until it tries to get the next link, and fails with that 50021:08
*** valw has quit IRC21:08
thomasemsigmavirus: I don't think this list check you have in your functional test is going to work for projects until we have a guaranteed order, since these are UUIDs.21:16
thomasemthe assertListEqual comparing the list of 30 from the GET /v1/projects21:16
thomasemSo, I'm thinking we can reduce scope to ensure we're just getting 30 back21:17
thomasemOnce we have a sort_key and such, we could then influence the ordering directly, but I don't think that's supported there yet?21:17
thomasemOr is it and I'm just silly21:18
jimbakerthomasem, maybe generate sequential UUIDs?21:18
jimbakerthomasem, that's the other change, which is modestly broken: https://review.openstack.org/#/c/434506/21:18
jimbakerwhen i state "maybe gen seq UUIDs"21:18
jimbakeri don't know if that's possible :)21:18
jimbakercertainly a useful testing idea. otherwise, crazy talk21:18
thomasemIT's possible. Practical, though?21:19
jimbaker[uuid.uuid1() for i in range(60)]21:22
jimbakershould maintain sort order, even though the specific UUIDs will change over time21:22
*** VW has quit IRC21:23
jimbakerthomasem, right, all the high bits are the from current time21:25
jimbakerso we could generate with some knowns in there too, like so: uuid.uuid1(42, 47)21:26
jimbakerbut additional calls will generate ones that are successively sorted higher21:27
jimbakerso that looks good to me21:27
*** VW has joined #craton21:27
jimbakerdid i say i was going to go running? ;)21:27
jimbakerok, biab21:28
thomasemjimbaker: I don't think that's guaranteed sequential.21:52
*** VW has quit IRC21:53
*** valw has joined #craton21:56
*** valw has quit IRC22:01
*** tojuvone has quit IRC22:03
*** jovon has joined #craton22:11
jimbakerthomasem, indeed: http://stackoverflow.com/questions/8713873/is-python-uuid1-sequential-as-timestamps22:13
jimbakerthomasem, so what about doing this: [uuid.UUID('00000000-0000-0000-0000-0000000000%02x' % i) for i in range(60)]22:23
jimbakerwe may need to set the uuid variant differently, but python does allow this construction (i believe in my limited reading, this bit pattern is an obsolete variant, so perfect for testing)22:25
*** VW has joined #craton22:35
*** VW_ has joined #craton22:37
*** VW has quit IRC22:37
thomasemjimbaker: lemme try... first gotta get the silly mock to work properly. Seems to be breaking something else. This all feels quite brittle, lol.22:39
thomasemNot wild about mocking in the functional tests22:39
thomasemWe will replace this as soon as we can sort.22:39
thomasemAnd I must be paying off some bad karma or something... internet went out for like 30 minutes.22:39
thomasemtotal my car, inundate me with insurance phone calls and paperwork, then cut my internet right when I'm in flow on something.22:40
*** VW has joined #craton22:40
*** VW_ has quit IRC22:40
*** VW has quit IRC22:42
thomasemBut, jimbaker,  I, too, got a run in. So go us.22:45
thomasemAlright, I'm going to put this silly patch to bed.22:46
jimbakerthomasem, awesome. runs are good, especially when sprinting ;)22:50
thomasemLol, indeed!22:50
jimbakerthomasem, i don't know about karma, but first step to getting my roof replaced with insurance adjuster stopping by (after significant wind damage last friday)22:52
thomasemyikes22:53
thomasemYeah, I remember you mentioning22:53
jimbakerfriday: new appliances, refrigerator (started to fail sunday) + electric range (has been wonky, so decided to just get that replaced too)22:53
thomasemDamn22:54
jimbakerhopefully that's enough for now!22:54
thomasemHopefully!22:56
thomasemjimbaker: I can't mock this because it's talking to the API in the container in the functional tests...23:03
thomasemI think the best bet here is to wait for sorting to be available, really.23:04
thomasemHow about I assert the links, exhaust the pagination, and then check that the lists contain the same items?23:05
*** _d34dh0r53_ is now known as d34dh0r5323:09
jimbakerthomasem, +123:10
*** valw has joined #craton23:44
*** valw has quit IRC23:49
thomasemBuhh... that way is broken by the bug you referenced earlier.23:51
thomasemOkay, since this patch is large enough, I vote we assert just the count for now, and follow up with a more robust test once sorting and such is fixed.23:52
thomasemjimbaker: ^^23:52
thomasemSince the next link includes sort_key and sort_dir23:53
thomasemEspecially since that's an unrelated bug.23:53
thomasemAs this patch aims to add in project variables support23:54
thomasemnot pagination23:54
thomasemReady for review: https://review.openstack.org/#/c/42777723:56
thomasemI will file bugs for the problems that have been found in the course of all of this.23:57
thomasemJust making notes and I'll flesh it out more tomorrow as it's getting really late for me.23:58
thomasemhave a lovely evening, everyone23:58
thomasem(or morning, as the case may be)23:58

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