| *** VW has joined #craton | 00:03 | |
| jimbaker | thomasem, i can combine | 00:06 |
|---|---|---|
| jimbaker | let's just do that | 00:06 |
| jimbaker | so 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 minors | 00:06 |
| jimbaker | the other issue is that the validation does not allow sort_keys and sort_dir to be specified, but again this is an easy fix | 00:07 |
| jimbaker | (at least for /v1/hosts) | 00:07 |
| *** VW has quit IRC | 00:08 | |
| jimbaker | thomasem, ok, i hope no pitchforks for my approving https://review.openstack.org/#/c/426229 | 00:46 |
| jimbaker | but it seemed the right thing to do imho | 00:46 |
| jimbaker | given no one is actually yet consuming those links | 00:47 |
| jimbaker | of 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 FYI | 01:01 |
| jimbaker | move fast, break things??? | 01:01 |
| jimbaker | back to curl for the moment :) | 01:01 |
| *** Syed__ has quit IRC | 02:45 | |
| *** serverascode_ has joined #craton | 03:05 | |
| *** keekz_ has joined #craton | 03:06 | |
| *** _d34dh0r53_ has joined #craton | 03:06 | |
| *** keekz has quit IRC | 03:07 | |
| *** serverascode has quit IRC | 03:07 | |
| *** d34dh0r53 has quit IRC | 03:07 | |
| *** keekz_ is now known as keekz | 03:07 | |
| *** serverascode_ is now known as serverascode | 03:18 | |
| *** tojuvone has joined #craton | 07:26 | |
| *** mdorman has quit IRC | 11:53 | |
| *** mdorman has joined #craton | 11:55 | |
| sigmavirus | jimbaker: the bug you filed for the client is a dupe | 13:11 |
| sigmavirus | there's a separate project for client bugs | 13:11 |
| sigmavirus | jimbaker: also, are you planningon picking up the clientn work I had started or?? | 13:26 |
| *** VW has joined #craton | 13:56 | |
| jimbaker | sigmavirus, sorry, i forgot we tracked distinctly when i filed, doh. thanks for fixing that dupe | 14:04 |
| sigmavirus | jimbaker: no worries :) | 14:04 |
| jimbaker | sigmavirus, i do have an outstanding fix for the client/CLI in https://review.openstack.org/#/c/434068/ | 14:04 |
| jimbaker | it's modest | 14:04 |
| thomasem | jimbaker: 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 |
| jimbaker | and maybe there is a better way of doing things | 14:05 |
| jimbaker | thomasem, i suppose i believe everyone gets to enjoy some merge conflicts | 14:05 |
| thomasem | Of course. That's the nature of it. I'm only concerned about the priorities and this being delayed further as a result. | 14:06 |
| sigmavirus | jimbaker: that doesn't handle pagination for the users :) My patch was focusing on that | 14:06 |
| jimbaker | i understand that pagination had lower priority. but it does work | 14:06 |
| sigmavirus | thomasem: you're right | 14:06 |
| sigmavirus | thomasem: do you want help with the merge-conflict? | 14:06 |
| sigmavirus | I am ... kind of experienced now at handling them quickly | 14:07 |
| thomasem | Nah, it's cool. I'm digging in right now. | 14:07 |
| thomasem | Lol, same. :) | 14:07 |
| thomasem | The 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 |
| jimbaker | i assume that the minors identified by thomasem in sigmavirus' patch were all because of merge conflict fixes | 14:08 |
| jimbaker | thomasem, the other thing we got in the deal is the links support | 14:08 |
| thomasem | I get that, and do appreciate it. I will shut up now. | 14:09 |
| jimbaker | merge conflict fixes/not quite merge conflict fixes | 14:09 |
| thomasem | Oh, yeah. Maybe they were? | 14:10 |
| thomasem | Was that regressed code to an earlier state or something? | 14:10 |
| jimbaker | thomasem, they have the classical presentation of a merge regression | 14:10 |
| thomasem | Anyway, the minors I identified do not actually cause a bug or anything. It was just extra code. | 14:10 |
| jimbaker | indeed. tiny amount of tech debt compared to progress attained. ruthless i am | 14:11 |
| jimbaker | ;) | 14:11 |
| thomasem | Yeah | 14:11 |
| jimbaker | so 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 fixed | 14:13 |
| sigmavirus | thomasem: to be fair, I really wasn't expecting the pagination work to get merged | 14:18 |
| thomasem | It'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 |
| thomasem | Ultimately, I'm glad it did. It is a very useful thing to have. | 14:20 |
| sigmavirus | thomasem: no don't worry about your tone | 14:20 |
| sigmavirus | I've been saying the same things since before you got here | 14:21 |
| thomasem | Lol, 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 |
| thomasem | LOL | 14:26 |
| thomasem | That's perfectly fair | 14:26 |
| thomasem | sigmavirus: 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 |
| thomasem | Since it seems projects_get_by_name wasn't tested before | 14:28 |
| sigmavirus | hm | 14:28 |
| thomasem | It's expecting the links to be returned as well as the query result | 14:28 |
| sigmavirus | That looks like git did something "helpful" when resolving a conflict | 14:28 |
| thomasem | Bummer | 14:28 |
| sigmavirus | https://i.imgur.com/26k9Td7.gif | 14:29 |
| thomasem | Lol | 14:30 |
| thomasem | Alright, cool. I'll just add back what was there. | 14:30 |
| jimbaker | everyone gets something with a breaking change :) | 14:30 |
| thomasem | Well, fortunately this wont' happen again, because tests! | 14:30 |
| thomasem | won't* | 14:30 |
| jimbaker | we hope. the tests are getting better... | 14:31 |
| thomasem | patch-by-patch | 14:31 |
| thomasem | 'tis all you can do | 14:31 |
| *** jovon has joined #craton | 14:34 | |
| sigmavirus | thomasem: added a change for that | 14:43 |
| sigmavirus | working on tests | 14:43 |
| thomasem | sigmavirus: oh, I was fixing it in my patch already... | 14:44 |
| sigmavirus | I need to turn notifications on for this channel (I think we can do it now) | 14:45 |
| thomasem | Lol | 14:45 |
| sigmavirus | (Gerrit -> IRC notifications) | 14:45 |
| sigmavirus | I started hacking on it when you pointed it out | 14:45 |
| thomasem | I 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 |
| thomasem | sigmavirus: ^^ and, yep, notifications would be nice | 14:47 |
| thomasem | What on earth? It's not executing my craton/db/sqlalchemy/api.py in the tests... | 14:50 |
| thomasem | I blew away the entire directory and it's still not | 14:50 |
| thomasem | Ohhhhhhhhhhhhhhhh | 14:53 |
| thomasem | I'm a baddie. That's why. | 14:53 |
| sigmavirus | oh? | 14:54 |
| sigmavirus | I'm adding a functional test at the moment because that's slightly easier | 14:54 |
| thomasem | Didn't fix my mock up properly. | 14:54 |
| thomasem | Nothing to see here... | 14:54 |
| thomasem | I'm ashamed now. | 14:54 |
| * sigmavirus thinks we use mock too much in our db tests | 14:54 | |
| * sigmavirus is uncool with the popular crowd | 14:55 | |
| sigmavirus | while my functional tests run, I'll brb | 14:55 |
| tojuvone | Evening Craton & friends | 15:03 |
| thomasem | Alright, fixed up and running functional tests also. | 15:10 |
| thomasem | hey tojuvone! | 15:10 |
| jimbaker | tojuvone, welcome back. great vacation in london? | 15:11 |
| tojuvone | jimbaker, Thanks, as my son said, best trip ever :) | 15:12 |
| tojuvone | but very cold | 15:12 |
| jimbaker | no doubt, a harry potter fan | 15:12 |
| tojuvone | btw, I put some thought here: https://tomijuvonen.blog/2017/02/10/planned-host-maintenance/ | 15:14 |
| tojuvone | Also there is a mail from Curtis in Openstack-operators mailing list. Thought to respond. | 15:19 |
| sigmavirus | thomasem: so if you want to throw your db test onto my change, that would be good | 15:20 |
| sigmavirus | Hi tojuvone! | 15:20 |
| sigmavirus | https://review.openstack.org/434319 | 15:20 |
| *** Syed__ has joined #craton | 15:24 | |
| thomasem | sigmavirus: done and pushed | 15:31 |
| sigmavirus | :cake: | 15:31 |
| sigmavirus | I use GitHub emoji as if they work everywhere =P | 15:31 |
| thomasem | :wookieedance: | 15:31 |
| sigmavirus | Wait, is that a GitHub Emoji? | 15:32 |
| thomasem | I use custom Slack emoji's as if they work everywhere. | 15:32 |
| sigmavirus | ah | 15:32 |
| sigmavirus | haha | 15:32 |
| thomasem | It ought to be, imo. | 15:32 |
| thomasem | Why it isn't yet is so far beyond me. | 15:32 |
| thomasem | jimbaker: can we get a review of this https://review.openstack.org/#/c/434319? | 15:33 |
| jimbaker | thomasem, sure | 15:33 |
| thomasem | quick change since it's broken now | 15:33 |
| jimbaker | we are going to get that in before the project vars, which i'm reviewing now and about to -1 | 15:33 |
| jimbaker | since i found a bit of brokeness in it | 15:33 |
| thomasem | That's fine. It's dependent anyway now. | 15:34 |
| jimbaker | let me finish that review first, then i will review sigmavirus' (and yours) | 15:34 |
| jimbaker | cool | 15:34 |
| jimbaker | thomasem, reviewed project vars. do take a look at the logger as well | 15:38 |
| jimbaker | but you should have everything you need to reproduce from my review | 15:38 |
| thomasem | What on earth | 15:39 |
| thomasem | Thanks jimbaker, looking into it. Something went sideways | 15:39 |
| jimbaker | possibly a merge problem | 15:39 |
| thomasem | Wouldn't be surprised | 15:39 |
| jimbaker | whatever it is, should be easy to fix | 15:39 |
| jimbaker | i'm more concerned about the logger setup | 15:40 |
| jimbaker | or at least equally concerned | 15:40 |
| jimbaker | incidentally this is a good example of where we need to do a better job of producing json | 15:40 |
| jimbaker | i am now in a habit of always doing this | 15: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 |
| jimbaker | in other places we do return a JSON body in case of error | 15:42 |
| thomasem | The API WG guidelines say to return JSON errors, iirc. | 15:42 |
| jimbaker | thomasem, yep. so spit & polish :) | 15:43 |
| jimbaker | should be easy enough to ensure, much like with returning indent=2 and sort_keys=True for the json response | 15:44 |
| jimbaker | will file a bug | 15:45 |
| thomasem | What you found is something to do with the `demo` user | 15:45 |
| thomasem | using bootstrap it works just fine | 15:45 |
| jimbaker | thomasem, but the demo user should be created by the bootstrap process so it just works, right? | 15:48 |
| jimbaker | my assumption is that bootstrap is just there to get us to a REST API createable projects/users | 15:48 |
| thomasem | I meant the bootstrap user. | 15:49 |
| thomasem | the bootstrap user works fine, demo does not. | 15:49 |
| thomasem | jimbaker: ^^ | 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 |
| thomasem | A 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 |
| jimbaker | right. so we should make sure the demo user does work as expected | 15:50 |
| thomasem | which makes precisely 0 sense to me. | 15:50 |
| jimbaker | i don't expect the bootstrap user to work at all :) | 15:50 |
| thomasem | I get that. I'm more perplexed by how that even HAS an impact here. | 15:50 |
| thomasem | Some weird side effect | 15:50 |
| jimbaker | yep. so anyway... code | 15:51 |
| jimbaker | ;) | 15:51 |
| thomasem | ? | 15:51 |
| sigmavirus | heh | 15:51 |
| jimbaker | code can be weird | 15:51 |
| jimbaker | until we figure it out | 15:51 |
| thomasem | Naturally | 15:51 |
| thomasem | And then it can still be weird because it's implemented poorly! :D | 15:51 |
| jimbaker | hopefully our layers are not so complex here. it would not seem to be the case | 15:51 |
| jimbaker | :) | 15:51 |
| jimbaker | but not so here i think | 15:51 |
| thomasem | Mmmkay | 15:52 |
| jimbaker | thomasem, ok, hopefully this small starter "project" i assigned you will complete at some point | 15:52 |
| jimbaker | clearly we have realized it was a little more than that | 15:52 |
| jimbaker | and yes, i think it is close | 15:53 |
| jimbaker | to being done | 15:53 |
| thomasem | Yeah, that's no problem. Lots of problems are icebergs. | 15:53 |
| jimbaker | indeed | 15:53 |
| thomasem | So, 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 |
| thomasem | Alright, so this is even more complicated, then | 16:06 |
| thomasem | jimbaker: This is due to the generic variable stuff necessitating removing the admin check for this function | 16:07 |
| jimbaker | thomasem, yep, that would make sense | 16:07 |
| jimbaker | https://bugs.launchpad.net/craton/+bug/1665015 | 16:07 |
| openstack | Launchpad bug 1665015 in craton "All text errors reported by the Craton REST API should be JSON encoded" [High,New] | 16:07 |
| thomasem | So, 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 up | 16:07 |
| *** VW_ has joined #craton | 16:09 | |
| thomasem | Thinking about how we want to support that (or work around it), then. | 16:09 |
| *** VW_ has quit IRC | 16:09 | |
| jimbaker | thomasem, 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 patch | 16:10 |
| *** VW_ has joined #craton | 16:10 | |
| thomasem | jimbaker: well, we could give demo user admin temporarily | 16:10 |
| thomasem | I 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 |
| jimbaker | thomasem, we will want to do the shim first. rbac is post cmdb milestone | 16:11 |
| thomasem | Yeah | 16:11 |
| thomasem | Just trying to conceive what that might look like. | 16:11 |
| jimbaker | shim should be reasonable. just need to add the necessary context stuff in the generic variables support | 16:12 |
| thomasem | Thinking a list of resources mapped to their permissions | 16:12 |
| *** VW has quit IRC | 16:12 | |
| jimbaker | mini-rbac | 16:12 |
| thomasem | Basically | 16:12 |
| thomasem | Not my favorite, but until we have proper RBAC, it'll have to do. | 16:13 |
| jimbaker | so just to be clear, we already have all the necessary info to do mini-rbac | 16:13 |
| jimbaker | in the schema | 16:13 |
| thomasem | Yes, it's a matter of leveraging it. | 16:13 |
| jimbaker | it just needs to be used by that generic vars support. so yeah, let's just separate and get your work in | 16:13 |
| jimbaker | ok, i'm going to +2 now unless we can think of an objection | 16:14 |
| jimbaker | (or we can wait 30 min...) | 16:14 |
| thomasem | jimbaker: 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 |
| thomasem | That can all happen within 30 min | 16:18 |
| thomasem | I also have the LOG fix in, so | 16:19 |
| thomasem | that logs properly now and sends a JSON error | 16:19 |
| thomasem | Or 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 |
| thomasem | It's either/or, really | 16:24 |
| sigmavirus | thomasem: you can rebase your patch on top of mine | 16:26 |
| sigmavirus | and gerrit will understand | 16:26 |
| * sigmavirus isn't sure how familiar you are with gerrit/git-review | 16:26 | |
| thomasem | I'm getting there. It's been a long time since I've used it. :) | 16:28 |
| *** VW_ has quit IRC | 16:30 | |
| *** VW has joined #craton | 16:30 | |
| sigmavirus | so 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 that | 16: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 |
| thomasem | Nice trick. Thank you. | 16:32 |
| jimbaker | sigmavirus, thanks for the beta | 16:38 |
| jimbaker | (using the climbing term) | 16:39 |
| * sigmavirus doesn't climb | 16:39 | |
| jimbaker | anyway, 5 min in vidyo. quick dog walk | 16:39 |
| sigmavirus | what about vidyo? | 16:39 |
| farid | sigmavirus: you should have an invite, do you not ? | 16:42 |
| farid | guess not | 16:43 |
| farid | you do now :) | 16:43 |
| sigmavirus | hah | 16:44 |
| sigmavirus | thomasem: after destroying some dangling (cached) images, i can reproduce the functional failures | 16:51 |
| sigmavirus | Are you already tackling those or should I go for it? | 16:52 |
| thomasem | Ahhhh, gotcha | 16:52 |
| thomasem | Yeah, it's a permissions problem. I'll tackle it. I already addressed it in my patch for my other functional tests. | 16:52 |
| thomasem | Matter of doing the same thing here. | 16:52 |
| sigmavirus | Got it | 16:52 |
| sigmavirus | Oh I should go add a functional test gate to our project | 16:52 |
| thomasem | +100 | 16:52 |
| sigmavirus | https://review.openstack.org/434391 < Gerrit -> IRC notifications | 17:06 |
| sigmavirus | https://review.openstack.org/434399 < Functional testing | 17:06 |
| jimbaker | +1000 | 17:09 |
| jimbaker | i take your +100, and raise it exponentially :) | 17:10 |
| jimbaker | but it is an annoying aspect of the review process to have to do tox -e functional. i rather focus on kicking the tires | 17:11 |
| jimbaker | (if indeed that's what people do. because kicking tires sounds painful) | 17:11 |
| sigmavirus | jimbaker: only kick tires when wearing steel-toe boots | 17:15 |
| jimbaker | sigmavirus, got it. sounds like a new test tool name | 17:17 |
| sigmavirus | lol | 17:18 |
| jimbaker | speaking of irc notifications, https://review.openstack.org/434399 needs a minor fix | 17:22 |
| jimbaker | sigmavirus, ^^^ | 17:23 |
| jimbaker | and given it's been raised to +1000 as something we could benefit from :), just mentioning it | 17:24 |
| sigmavirus | thanks jimbaker | 17:24 |
| sigmavirus | jimbaker: should be fixed up now | 17:25 |
| thomasem | sigmavirus: 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 |
| sigmavirus | thomasem: the three patches? | 17:33 |
| * sigmavirus shrugs works for me | 17:33 | |
| thomasem | ALL THE PATCHES | 17:34 |
| thomasem | Squash them all. Mega review. | 17:34 |
| sigmavirus | thomasem: I've seen that before :( | 17:34 |
| thomasem | Oh dear | 17:34 |
| thomasem | I'm sorry | 17:34 |
| sigmavirus | It'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 -x | 17:35 |
| sigmavirus | Syed__: stick with me, I know way too many oddly specific tricks | 17:35 |
| Syed__ | sigmavirus: :) | 17:35 |
| jimbaker | git-harry, https://bugs.launchpad.net/craton/+bug/1665050 - now we have captured it, and can work when get there | 17:37 |
| openstack | Launchpad bug 1665050 in craton "Support alternative variable overrides" [Undecided,New] | 17:37 |
| jimbaker | it is explicitly NOT in scope for the cmdb milestone | 17:37 |
| jimbaker | even though it was brought up in our discussions, specifically with respect to cruton | 17:37 |
| thomasem | Come on functional tests, please pass this time. | 17:37 |
| jimbaker | the good thing is, we probably only get to suffer through this very much now and then. i hope. | 17:39 |
| thomasem | I hope as well. | 17:39 |
| thomasem | Surely this is shortening my life span. | 17:39 |
| thomasem | :P | 17:39 |
| Syed__ | haah | 17:39 |
| jimbaker | everyone, take deep breaths... | 17:40 |
| jimbaker | thomasem, so i assume you are fixing up https://review.openstack.org/#/c/434319/ ? | 17:46 |
| jimbaker | (as in, functional testing failing) | 17:47 |
| thomasem | jimbaker: 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/427777 | 17:50 |
| jimbaker | yes, let's just do that | 17:50 |
| thomasem | Reason being - for those functional tests to work, it has to rely on the bootstrap stuff I added. | 17:50 |
| jimbaker | exactly | 17:50 |
| thomasem | Excellent | 17:50 |
| *** valw has joined #craton | 17:51 | |
| * sigmavirus lunch | 17:55 | |
| *** VW has quit IRC | 18:12 | |
| *** VW has joined #craton | 18:12 | |
| *** VW has quit IRC | 18:13 | |
| *** VW has joined #craton | 18:13 | |
| thomasem | I need to lunch as well, I'll have to finish this when I get back. | 18:18 |
| jimbaker | another 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 |
| jimbaker | https://bugs.launchpad.net/craton/+bug/1665066 | 18:19 |
| openstack | Launchpad 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 #craton | 18:30 | |
| *** VW_ has quit IRC | 18:31 | |
| *** VW_ has joined #craton | 18:31 | |
| *** valw has quit IRC | 18:32 | |
| *** VW has quit IRC | 18:33 | |
| farid | jimbaker 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/playbooks | 18:34 |
| farid | currently getting some test data together though | 18:34 |
| *** valw has joined #craton | 18:35 | |
| jimbaker | farid, awesome | 18:37 |
| *** VW_ has quit IRC | 18:37 | |
| jimbaker | and thanks cloudnull | 18:37 |
| *** VW has joined #craton | 18:38 | |
| farid | jimbaker: got this for now https://gist.github.com/anonymous/0885ce1944523e0b78d898457b17f9e3 | 18:39 |
| farid | think it should be a good start | 18:40 |
| jimbaker | farid, very much so. we should add a link to that data in our cmdb etherpad | 18:42 |
| farid | yep will do | 18:45 |
| *** jovon has quit IRC | 18:52 | |
| mudpuppy | :) | 19:02 |
| *** valw has quit IRC | 19:03 | |
| sigmavirus | thanks mudpuppy :) | 19:08 |
| *** VW has quit IRC | 19:22 | |
| *** VW has joined #craton | 19:29 | |
| *** valw has joined #craton | 19:31 | |
| *** valw has quit IRC | 19:35 | |
| *** valw has joined #craton | 19:51 | |
| sigmavirus | https://review.openstack.org/#/c/434399 is passing now | 20:02 |
| sigmavirus | I'm just testing https://review.openstack.org/#/c/429040/ on my dev env | 20:02 |
| sigmavirus | jimbaker: thomasem y'all around? | 20:19 |
| jimbaker | sigmavirus, so we can do https://review.openstack.org/#/c/429040/ instead of my patch? | 20:27 |
| sigmavirus | yep, it does more and adds tests | 20:28 |
| sigmavirus | my problem testing it is that our json-schema doesn't allow sort-dir/sort-keys | 20:28 |
| sigmavirus | so | 20:28 |
| sigmavirus | I'm going to go ahead and change the schema | 20:28 |
| sigmavirus | Just wanted to make sure no one had serious objections to that | 20:28 |
| jimbaker | sigmavirus, please make those changes to the schema | 20:29 |
| jimbaker | that's the essence of my bug report in https://bugs.launchpad.net/craton/+bug/1664786 | 20:30 |
| openstack | Launchpad bug 1664786 in craton "Fix minor problems identified in recent pagination work" [Critical,New] - Assigned to Ian Cordasco (icordasc) | 20:30 |
| jimbaker | specifically in my comments about the schema in https://review.openstack.org/#/c/426229/11 | 20:30 |
| jimbaker | otherwise, pagination links don't work, etc | 20:30 |
| jimbaker | sigmavirus, and in the meantime, let me review that client work; and i will abandon my patch | 20:31 |
| jimbaker | any reason why https://review.openstack.org/#/c/434444/ hasn't merged yet? | 20:32 |
| jimbaker | and https://review.openstack.org/#/c/434433/? | 20:32 |
| sigmavirus | it's not on 434444 | 20:33 |
| sigmavirus | http://status.openstack.org/zuul/ | 20:33 |
| sigmavirus | let's see if adding a new vote to it updates it | 20:34 |
| Syed__ | yeah thats what i checked | 20:34 |
| Syed__ | it doesn't show it to be at zuul | 20:34 |
| Syed__ | Same with 434433 | 20:34 |
| sigmavirus | Honestly I think zuul is struggling right now for some reason | 20:35 |
| sigmavirus | I saw a lot of "POST_FAILURE" results recently | 20:35 |
| jimbaker | sigmavirus, got it | 20:37 |
| sigmavirus | ah looks like logs.openstack.org is the struggler | 20:37 |
| sigmavirus | can't delete old files fast enough to keep up with the upload of new log files | 20:37 |
| jimbaker | sigmavirus, i like the generator in the client, very nice | 20:38 |
| thomasem | sigmavirus: I'm around now | 20:38 |
| sigmavirus | It also required no updates to work with print_list which was nice | 20:38 |
| jimbaker | just need to get the schema change in, but clearly it's generating the correct links before the api barfs | 20:39 |
| jimbaker | it = client | 20:39 |
| sigmavirus | yep | 20:40 |
| sigmavirus | testing the schema change locally first | 20:40 |
| sigmavirus | =P | 20:40 |
| jimbaker | not a problem | 20:42 |
| sigmavirus | I pushed it for review, but want to quickly check it | 20:43 |
| *** VW has quit IRC | 20:49 | |
| git-harry | jimbaker: it depends on https://review.openstack.org/#/c/434342/1 | 20:52 |
| jimbaker | sigmavirus, 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®ion_id=1&limit=30&sort_dir=asc&sort_keys=created_at%2Cid | 20:52 |
| sigmavirus | hm | 20:52 |
| jimbaker | sigmavirus, are you sure you don't want to generate this as &sort_keys=created_at&sort_keys=id | 20:52 |
| jimbaker | i'm pretty sure that works just fine | 20:53 |
| * sigmavirus wonders what is causing that 500 | 20:53 | |
| sigmavirus | oh | 20:53 |
| sigmavirus | I suspect we need some pre-processing on the keys | 20:53 |
| sigmavirus | I think the preferred manner is wht I have | 20:53 |
| jimbaker | yeah, because of the validation | 20:54 |
| sigmavirus | *API-WG manner | 20:54 |
| jimbaker | i'm good if we can flask-restful accept | 20:54 |
| jimbaker | can get | 20:54 |
| sigmavirus | that said, my testing shows host-list working fine | 20:54 |
| sigmavirus | hm | 20:54 |
| sigmavirus | weird | 20:55 |
| sigmavirus | oh | 20:55 |
| sigmavirus | let me try adding sort keys | 20: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 | ^ jimbaker | 20:55 |
| jimbaker | git-harry, ahh, ok, i will take a look | 20:55 |
| jimbaker | another important change | 20:56 |
| *** VW has joined #craton | 20:58 | |
| sigmavirus | jimbaker: updated with some processing on sort_keys | 20:58 |
| jimbaker | sigmavirus, awesome | 21:00 |
| sigmavirus | having uninterrupted time is surprisingly good for productivitty | 21:02 |
| * sigmavirus is heading out | 21:03 | |
| sigmavirus | later yinz | 21:03 |
| jimbaker | sigmavirus, ok if i update https://review.openstack.org/#/c/434506/ given conflicts with git-harry's recent work? | 21:04 |
| sigmavirus | gopher it | 21:05 |
| * jimbaker wants everything working end-to-end again for some reason :) | 21:05 | |
| jimbaker | thanks! | 21:05 |
| jimbaker | sigmavirus, i'm still getting this: TypeError: InvalidSortKey('Sort key supplied is invalid: created_at,id',) is not JSON serializable | 21:06 |
| jimbaker | 127.0.0.1 - - [15/Feb/2017 14:05:56] "GET /v1/hosts?marker=10&limit=30&sort_dir=asc®ion_id=1&sort_keys=created_at%2Cid HTTP/1.1" 500 59 | 21:06 |
| jimbaker | ok, we will just wait a bit longer | 21:06 |
| jimbaker | i'm going to get a run in. as usual, late | 21:06 |
| jimbaker | fwiw, i test this by going into the client, and doing this: list(craton.hosts.list(region_id=1)) | 21:08 |
| jimbaker | so that generator works fine until it tries to get the next link, and fails with that 500 | 21:08 |
| *** valw has quit IRC | 21:08 | |
| thomasem | sigmavirus: 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 |
| thomasem | the assertListEqual comparing the list of 30 from the GET /v1/projects | 21:16 |
| thomasem | So, I'm thinking we can reduce scope to ensure we're just getting 30 back | 21:17 |
| thomasem | Once 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 |
| thomasem | Or is it and I'm just silly | 21:18 |
| jimbaker | thomasem, maybe generate sequential UUIDs? | 21:18 |
| jimbaker | thomasem, that's the other change, which is modestly broken: https://review.openstack.org/#/c/434506/ | 21:18 |
| jimbaker | when i state "maybe gen seq UUIDs" | 21:18 |
| jimbaker | i don't know if that's possible :) | 21:18 |
| jimbaker | certainly a useful testing idea. otherwise, crazy talk | 21:18 |
| thomasem | IT's possible. Practical, though? | 21:19 |
| jimbaker | [uuid.uuid1() for i in range(60)] | 21:22 |
| jimbaker | should maintain sort order, even though the specific UUIDs will change over time | 21:22 |
| *** VW has quit IRC | 21:23 | |
| jimbaker | thomasem, right, all the high bits are the from current time | 21:25 |
| jimbaker | so we could generate with some knowns in there too, like so: uuid.uuid1(42, 47) | 21:26 |
| jimbaker | but additional calls will generate ones that are successively sorted higher | 21:27 |
| jimbaker | so that looks good to me | 21:27 |
| *** VW has joined #craton | 21:27 | |
| jimbaker | did i say i was going to go running? ;) | 21:27 |
| jimbaker | ok, biab | 21:28 |
| thomasem | jimbaker: I don't think that's guaranteed sequential. | 21:52 |
| *** VW has quit IRC | 21:53 | |
| *** valw has joined #craton | 21:56 | |
| *** valw has quit IRC | 22:01 | |
| *** tojuvone has quit IRC | 22:03 | |
| *** jovon has joined #craton | 22:11 | |
| jimbaker | thomasem, indeed: http://stackoverflow.com/questions/8713873/is-python-uuid1-sequential-as-timestamps | 22:13 |
| jimbaker | thomasem, so what about doing this: [uuid.UUID('00000000-0000-0000-0000-0000000000%02x' % i) for i in range(60)] | 22:23 |
| jimbaker | we 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 #craton | 22:35 | |
| *** VW_ has joined #craton | 22:37 | |
| *** VW has quit IRC | 22:37 | |
| thomasem | jimbaker: 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 |
| thomasem | Not wild about mocking in the functional tests | 22:39 |
| thomasem | We will replace this as soon as we can sort. | 22:39 |
| thomasem | And I must be paying off some bad karma or something... internet went out for like 30 minutes. | 22:39 |
| thomasem | total 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 #craton | 22:40 | |
| *** VW_ has quit IRC | 22:40 | |
| *** VW has quit IRC | 22:42 | |
| thomasem | But, jimbaker, I, too, got a run in. So go us. | 22:45 |
| thomasem | Alright, I'm going to put this silly patch to bed. | 22:46 |
| jimbaker | thomasem, awesome. runs are good, especially when sprinting ;) | 22:50 |
| thomasem | Lol, indeed! | 22:50 |
| jimbaker | thomasem, 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 |
| thomasem | yikes | 22:53 |
| thomasem | Yeah, I remember you mentioning | 22:53 |
| jimbaker | friday: new appliances, refrigerator (started to fail sunday) + electric range (has been wonky, so decided to just get that replaced too) | 22:53 |
| thomasem | Damn | 22:54 |
| jimbaker | hopefully that's enough for now! | 22:54 |
| thomasem | Hopefully! | 22:56 |
| thomasem | jimbaker: I can't mock this because it's talking to the API in the container in the functional tests... | 23:03 |
| thomasem | I think the best bet here is to wait for sorting to be available, really. | 23:04 |
| thomasem | How 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 d34dh0r53 | 23:09 | |
| jimbaker | thomasem, +1 | 23:10 |
| *** valw has joined #craton | 23:44 | |
| *** valw has quit IRC | 23:49 | |
| thomasem | Buhh... that way is broken by the bug you referenced earlier. | 23:51 |
| thomasem | Okay, 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 |
| thomasem | jimbaker: ^^ | 23:52 |
| thomasem | Since the next link includes sort_key and sort_dir | 23:53 |
| thomasem | Especially since that's an unrelated bug. | 23:53 |
| thomasem | As this patch aims to add in project variables support | 23:54 |
| thomasem | not pagination | 23:54 |
| thomasem | Ready for review: https://review.openstack.org/#/c/427777 | 23:56 |
| thomasem | I will file bugs for the problems that have been found in the course of all of this. | 23:57 |
| thomasem | Just making notes and I'll flesh it out more tomorrow as it's getting really late for me. | 23:58 |
| thomasem | have a lovely evening, everyone | 23: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!
