*** rainya_ has quit IRC | 00:14 | |
*** VW has joined #craton | 00:26 | |
*** VW has quit IRC | 00:26 | |
*** VW has joined #craton | 00:27 | |
*** rainya has joined #craton | 00:54 | |
*** Syed__ has quit IRC | 01:25 | |
*** rainya has quit IRC | 01:41 | |
*** VW has quit IRC | 01:44 | |
*** git-harry has quit IRC | 01:49 | |
*** git-harry has joined #craton | 01:51 | |
*** VW has joined #craton | 02:09 | |
jimbaker | the test failures are occurring because we changed how we were testing (independently), but we still need to get the update in. i expect sulo will have this fixed shortly with this change that he's working on: https://review.openstack.org/#/c/408784/ | 02:34 |
---|---|---|
jimbaker | (now independently, to be more precise) | 02:34 |
*** Bluebell_ has joined #craton | 03:35 | |
*** Bluebell_ has left #craton | 03:58 | |
tojuvone | Morning | 04:17 |
*** VW has quit IRC | 04:18 | |
*** VW has joined #craton | 04:20 | |
*** VW has quit IRC | 04:38 | |
*** VW has joined #craton | 04:38 | |
*** VW has quit IRC | 04:43 | |
*** VW has joined #craton | 07:39 | |
*** VW has quit IRC | 07:43 | |
sulo | https://bugs.launchpad.net/craton/+bug/1660922 | 09:24 |
openstack | Launchpad bug 1660922 in craton "Resource search by variable is broken" [High,New] - Assigned to Sulochan Acharya (sulochan-acharya) | 09:24 |
git-harry | sulo: two of the functional tests are failing for me, is that a regression or were they failing when committed? | 10:40 |
sulo | oh | 10:40 |
sulo | not sure .. nothign was failing afaik | 10:40 |
sulo | do you know which ones ? | 10:41 |
sulo | actaully let me try it out | 10:41 |
git-harry | craton.tests.functional.test_region_calls.APIV1RegionTest.test_region_get_details_for_region_1 and craton.tests.functional.test_region_calls.APIV1RegionTest.test_regions_get_all fail for me | 10:41 |
sulo | ah | 10:42 |
sulo | thats probably something else | 10:42 |
sulo | https://review.openstack.org/#/c/408784/ not merged yet | 10:42 |
sulo | i need to fix that commit | 10:42 |
git-harry | Were they broken by changes to the test setup? | 10:43 |
sulo | yeah, thats what it looks like .. all the patches were in chain so its probably that | 10:44 |
git-harry | What is required to get the functional testing enabled in the gate? Is there any reason it's not already enabled? | 10:45 |
sulo | git-harry: i think we should enable it .. just that no one done it yet | 10:45 |
sulo | git-harry: also, we should enable coverage gen and look at whats missing | 10:46 |
sulo | found a big bug on afeature we needed because things changed and we didnt | 10:46 |
sulo | have enough tests to cover that condition | 10:46 |
git-harry | sounds good | 10:47 |
git-harry | getting the stuff enabled that is, not the bug. | 10:47 |
sulo | jimbaker: sigmavirus: git-harry please check openstack-dev ml for my email and +/- accordingly, thx | 11:09 |
*** odyssey4me has left #craton | 11:17 | |
*** VW has joined #craton | 13:08 | |
*** VW has quit IRC | 13:08 | |
*** VW has joined #craton | 13:08 | |
*** VW has quit IRC | 13:09 | |
*** VW has joined #craton | 13:10 | |
*** Mitschke has quit IRC | 13:11 | |
*** VW has quit IRC | 13:14 | |
*** Mitschke has joined #craton | 13:49 | |
*** valw has joined #craton | 13:52 | |
*** VW has joined #craton | 13:55 | |
sulo | https://review.openstack.org/#/c/427717/ << vars filtering | 14:08 |
*** valw has quit IRC | 14:12 | |
*** valw has joined #craton | 14:13 | |
sigmavirus | sulo: tests are failing | 14:16 |
sulo | fixed | 14:18 |
*** valw has quit IRC | 14:36 | |
thomasem | o/ | 14:42 |
thomasem | sulo: in master or in review? | 14:45 |
sulo | thomasem: review | 15:06 |
thomasem | gotcha | 15:07 |
thomasem | Wanted to be sure I can still expect these two test failures in master, since I got them this morning. :D | 15:07 |
thomasem | Thanks! | 15:07 |
*** sigmavirus has quit IRC | 15:14 | |
sulo | thomasem: for functional tests ? if so there is pr up to fix that. | 15:26 |
*** sigmavirus has joined #craton | 15:26 | |
*** ChanServ sets mode: +o sigmavirus | 15:26 | |
sulo | actually, that needs to be updated | 15:26 |
sulo | let me do that now | 15:26 |
thomasem | sulo: yes, that's why I was asking. I think that patch needs a rebase. | 15:26 |
sulo | yeah, fixing that now | 15:26 |
thomasem | Woot | 15:26 |
thomasem | Thanks sulo | 15:26 |
*** jovon has joined #craton | 15:30 | |
sulo | ok rebased and updated https://review.openstack.org/#/c/408784/ | 15:43 |
thomasem | sulo: reviewing | 15:44 |
sulo | thanks, afk for a bit | 15:44 |
jimbaker | sulo, ack about your email | 15:56 |
*** VW has quit IRC | 15:56 | |
*** VW has joined #craton | 15:57 | |
jimbaker | sulo, thanks for the fixes on functional testing of regions. nice to see tox -e functional is working without problems again with this change | 16:01 |
jimbaker | next: we can make our functional tests stronger. but a future change! | 16:01 |
jimbaker | sulo, but first i guess we should address some coding aspects, per sigmavirus's comment. ok, i will add a few more things we should to do for https://review.openstack.org/#/c/408784 | 16:04 |
jimbaker | sigmavirus, the only doc i found of the codes requests supports is here: https://github.com/kennethreitz/requests/blob/master/requests/status_codes.py, although it is referenced of course in http://docs.python-requests.org/en/master/api/#status-code-lookup | 16:15 |
jimbaker | maybe a better doc somewhere else? | 16:15 |
sigmavirus | jimbaker: so if you do `from requests import status_codes as status; status.OK` | 16:15 |
sigmavirus | er | 16:15 |
jimbaker | for 422, https://github.com/kennethreitz/requests/blob/master/requests/status_codes.py#L57 | 16:15 |
sigmavirus | status.codes.OK | 16:15 |
sigmavirus | that'll work | 16:16 |
sigmavirus | status.codes.unprocessable | 16:16 |
sigmavirus | etc. | 16:16 |
sigmavirus | that will return 422 | 16:16 |
sigmavirus | but it's better not to write 422 explicitly IMO | 16:16 |
sigmavirus | the code name generally provides a good explanation in the test | 16:16 |
jimbaker | sure, that's all good. but i'm just saying it's not documented. the specific status code can be looked up from that source code; or by using ipython to autocomplete | 16:18 |
jimbaker | although the docs actually reference `requests.codes`, which doesn't autocomplete | 16:19 |
jimbaker | for some reason | 16:19 |
jimbaker | maybe some additional magic? | 16:19 |
jimbaker | anyway, i will attach my findings to the review | 16:20 |
sigmavirus | We do import the codes object into the requests namespace https://github.com/kennethreitz/requests/blob/master/requests/__init__.py#L67 | 16:20 |
sigmavirus | sulo: I'll be happy to update your review | 16:20 |
sigmavirus | I'm kind of hoping for the functional bases to be present before continuing on pagination work | 16:21 |
jimbaker | sounds good | 16:21 |
tojuvone | Hi guys | 16:22 |
sigmavirus | Hey tojuvone | 16:23 |
tojuvone | Was thinking if get Nova to link Craton, it would be totally cool for the project | 16:23 |
jimbaker | also if possible let's go be uniform with the assertions. i know it's often better to say assertX(first, second) (because no one remember the order), vs assertX(value, expected) (or is it the other way around?). but we keep on mixing it here in that file | 16:23 |
tojuvone | therefor tried to think a bit the namespace thing | 16:23 |
tojuvone | Put some here, what I got by fast: http://pastebin.com/NM33cfXu | 16:24 |
jimbaker | tojuvone, thanks. re setting variables in one call - yes, this is implemented atomically as a single db transaction | 16:25 |
jimbaker | we have to ensure that our notification process actually respects that of course, and doesn't send out notifications on a per row basis | 16:26 |
tojuvone | yes, would think notification payload would hold normally what in that namespace | 16:27 |
*** Syed__ has joined #craton | 16:27 | |
jimbaker | but i'm pretty sure right now that our rest apis consistently follow a unit of work - one rest api call is one db transaction | 16:27 |
tojuvone | then wondering how the hostid thing would work | 16:28 |
tojuvone | both admin and other user might not be allowed to get same variables | 16:29 |
jimbaker | so this is the first time we have discussed going from nova to craton to look up info, so that's an interesting twist | 16:30 |
jimbaker | but in general, if there is a unique identifier that we can look up a host on, we can just store it in craton. and then uniquely get the host back with a vars lookup (which sulo just fixed) | 16:31 |
tojuvone | well, I had this discussion before to some external thing owning this palnned host maintenance | 16:31 |
jimbaker | more importantly, the host id we use is opaque, so nova could store it if desired | 16:32 |
tojuvone | Now it would be Craton. | 16:32 |
jimbaker | right | 16:32 |
jimbaker | that's definitely something we would store - this is what we did with the similar inventory system in rackspace public cloud | 16:33 |
tojuvone | Tought that this might be in away now quite fance thing to figure out to people to start using Craton | 16:33 |
tojuvone | Nova do not want any extra data (been there). Meaning I would not expect anything more than what I put there | 16:35 |
tojuvone | I mean to store some "hostid" coming from Craton | 16:35 |
jimbaker | right, so we are definitely focused on storing this type of data. and doing further elaboration as we discussed about a set of maintenance variables | 16:35 |
tojuvone | Anyhow that hostid is already known by Nova | 16:36 |
sigmavirus | jimbaker: it's expected, value | 16:36 |
jimbaker | sigmavirus, thanks for the clarification :) | 16:37 |
*** valw has joined #craton | 16:37 | |
jimbaker | but first, same file consistency. even better, general consistency | 16:37 |
*** valw has quit IRC | 16:41 | |
jimbaker | sigmavirus, the other thing i would like to see in these tests is less assertions like self.assertEqual(2, len(resp.json())), and instead replace it with the expected data that is invariant (and perhaps check for existence of the others that do vary, such as create_time) | 16:44 |
jimbaker | in other words, this specific assertion is verifying that we got back two regions, but it's quite cryptic in its expectations | 16:45 |
sigmavirus | jimbaker: Yeah, I'm already working on that in the unit tests | 16:47 |
sigmavirus | That said, in the unit tests, where we mock out the dbapi, I feel conflicted | 16:47 |
jimbaker | well those specific tests are certainly super weak | 16:53 |
jimbaker | given that there's not so much surface to be tested once the dbapi functionality is mocked out | 16:53 |
*** VW has quit IRC | 17:01 | |
sigmavirus | jimbaker: right, I'm really not a fan of craton.tests.unit.test_api at all | 17:01 |
*** VW has joined #craton | 17:04 | |
tojuvone | So thinking when different things might be implemented... | 17:27 |
tojuvone | Maybe I should start soon drafting spec to Nova to have it in Pike to have something in time of Sydney | 17:28 |
tojuvone | Then just need to know how it can be in Craton side | 17:29 |
tojuvone | Good thing is that for Nova I have code quite ready if just get spec trough | 17:32 |
sigmavirus | tojuvone: propose the spec and code simultaneously and asap to allow it to have any chance of landing in Pike. | 17:34 |
sigmavirus | I would guess, that you'll get it in during Queens at the earliest though | 17:34 |
sigmavirus | I think Nova already has several goals set/specs set for Pike already | 17:34 |
*** valw has joined #craton | 17:38 | |
*** rainya has joined #craton | 17:41 | |
*** valw has quit IRC | 17:43 | |
jimbaker | right, even if it doesn't make pike, it's necessary to get the ball rolling now. if we can minimize changes in nova, and push more into craton, even better | 17:51 |
jimbaker | but we will do so by working out the spec | 17:51 |
jimbaker | and corresponding code | 17:52 |
jimbaker | tojuvone, ^^^ | 17:52 |
tojuvone | Sorry, was doing dishes :D | 17:56 |
tojuvone | Yes, luckily not very big change, but not the easiest either | 17:57 |
Syed__ | jimbaker: a question | 18:04 |
Syed__ | so i installed cratonclient on top of craton and trying to query some simple commands | 18:05 |
Syed__ | https://www.irccloud.com/pastebin/4s2vrvOl/ | 18:05 |
jimbaker | Syed__, i assume you have set your env vars | 18:05 |
*** harlowja_ has joined #craton | 18:05 | |
jimbaker | but if not, here's what i set | 18:05 |
Syed__ | hmm, its a local install, not through docker | 18:06 |
*** harlowja has quit IRC | 18:06 | |
Syed__ | so if i do a docker install , do i go into the docker container for craton and install cratonclient on that to play around ? | 18:06 |
Syed__ | or what should be the suggested approach in case of dockerize setup | 18:07 |
sulo | jimbaker: sigmavirus: just got back, reading scrollback | 18:07 |
jimbaker | Syed__, https://gist.github.com/jimbaker/559dd5c9d8933ea94a7e3798429fafcd | 18:07 |
jimbaker | Syed__, the Dockerfile instructions are precise | 18:08 |
jimbaker | but here's the script i use for starting and populating a craton instance with some fake data to play with | 18:08 |
jimbaker | https://gist.github.com/jimbaker/e74a7b98bc60519033fd455a22163ad2 | 18:08 |
Syed__ | got it, cool | 18:09 |
Syed__ | thanks | 18:09 |
jimbaker | once that is finished running, you should just be able to run the cli against it, without problems. it just works | 18:09 |
jimbaker | also worth pointing out that thomasem's note is only applicable if one is not running the Dockerfile, which does the user/project population via CMD ["tools/docker_run.sh"] | 18:11 |
jimbaker | i have other steps that i run if i want to run the db more directly | 18:11 |
thomasem | Yeah. I was going for a dev environment architecture I was more used to. And I intend for us to make changes to that as we approach something more production-y. For now, though, yeah... go with the prescribed method | 18:12 |
sulo | jimbaker: sigmavirus: so instead of 200, status_codes.code.ok for example ? | 18:13 |
sigmavirus | exactly sulo | 18:13 |
thomasem | My issues were related to two things - 1) trying to run functional tests on a Craton clone in Mac OS X using Docker for Mac, and 2) utilizing rsync and not handling file permissions properly between the source env and execution env. | 18:13 |
sigmavirus | If you want I can touch it up, assuming you're off for the rest of the day | 18:13 |
sulo | yeah i am around but not active right now | 18:15 |
sulo | sigmavirus: actually, i am a little confused with this, what do we gain from doing that ? | 18:17 |
thomasem | More reader-friendly code | 18:18 |
sigmavirus | sulo: having hard-coded status numbers in there is really gross | 18:19 |
thomasem | More idiomatic implementation | 18:19 |
sigmavirus | they end up being magic-ish numbers | 18:19 |
sulo | but they are status code .. the whole wide world uses them? | 18:19 |
sulo | i dont mind chaging it though | 18:19 |
jimbaker | thomasem, yeah, we definitely want to get to something that is production | 18:20 |
thomasem | jimbaker: we shall :) | 18:20 |
jimbaker | some fairly obvious approaches, but i think we can do it even better with a little containerization | 18:20 |
sulo | sigmavirus: thomasem: https://github.com/kennethreitz/requests/blob/f72684e13c5074a671506d29c1b5638156680ea7/tests/test_requests.py#L160 | 18:21 |
thomasem | Certainly has my vote. | 18:21 |
sigmavirus | sulo: if requests jumped off a bridge, would you do the same? | 18:21 |
sulo | sigmavirus: you are aksing to do this because of requests as per your comment | 18:21 |
thomasem | sulo: I want to point out, I personally have no problem using the hard-coded status code. It's a stylistic thing. | 18:21 |
sulo | sigmavirus: i am making a case that i dont see the point in doing this at all | 18:22 |
sigmavirus | sulo: no, I never said "because of requests" | 18:22 |
*** VW has quit IRC | 18:22 | |
sulo | sigmavirus: "We shouldn't hardcode numbers here for status codes. Requests has a way of referencing them by name" | 18:22 |
*** VW has joined #craton | 18:22 | |
sigmavirus | Yes, that's not "because requests does it" that's because it's convenient and easier to read to use the reason name | 18:22 |
sulo | anyhow, i dont see the point | 18:23 |
sulo | yall do the reviews, ill chage it if thats the vote | 18:23 |
thomasem | In situations like this, I'd be more inclined to merge until we make it a part of a style guide for the projec to set it as a standard that we utilize the requests status codes rather than the hard-coded one. Holding up a review over it doesn't seem like a good use of time. The main benefit I see is that you don't have to worry about what code means what. | 18:23 |
git-harry | sulo: I agree with you. I don't see the benefit. They have to be hard coded and using some other constant is still hard coding them. | 18:24 |
thomasem | Otherwise some reviews are going to get pinged for it, and others won't. | 18:24 |
thomasem | There's no controls around it. | 18:24 |
thomasem | All that said, we're taking more time talking about it than it takes to make the change now. | 18:25 |
thomasem | Hmmm, weird... doesn't seem to be picking up my schemas.py changes | 18:27 |
sigmavirus | thomasem: ? | 18:28 |
thomasem | I'm adding "projects_id_variables" and getting 500 when I try to talk to it for some reason. | 18:29 |
sigmavirus | thomasem: check craton-api.log | 18:29 |
thomasem | Yerp, digging in | 18:29 |
thomasem | Thanks | 18:29 |
sigmavirus | happy to hop onto a vidyo call if you need help | 18:29 |
thomasem | sigmavirus: I appreciate it! I'll letcha know if I have trouble figuring it out. :) | 18:30 |
*** rainya_ has joined #craton | 18:37 | |
*** rainya has quit IRC | 18:40 | |
jimbaker | i agree with sulo and thomasem - i was ready to approve the change https://review.openstack.org/#/c/408784/ because it actually fixes a broken build | 19:13 |
jimbaker | somehow we got ourselves into this with the recent set of changes, and a few mixups | 19:13 |
jimbaker | but without sulo's change, tox -e functional fails, and we have to know that it fails and workaround that in looking at things | 19:13 |
jimbaker | so much better if we make it incrementally better now; and also do the status code updates as well. also, we have the possibility of someone submitting a documentation PR to the requests project, because none of those status codes are explicitly documented | 19:14 |
jimbaker | (without reference to source code; the usage examples are not comprehensive) | 19:15 |
jimbaker | thomasem, yeah, the log output for the api server is pretty important. i think we are good about always logging errors. or print as necessary (that's my school of thought, it's rare i will be in pdb) | 19:17 |
*** VW has quit IRC | 19:18 | |
*** harlowja_ has quit IRC | 19:19 | |
jimbaker | ok, added my 2ยข to https://review.openstack.org/#/c/408784/ | 19:25 |
jimbaker | sigmavirus, sulo, thomasem, any objections for me +2-ing as well? | 19:26 |
jimbaker | git-harry, sorry, you were also in the conversation | 19:29 |
jimbaker | sigmavirus, also separately, please respond to sulo's email "[openstack-dev] [craton] Proposing Harry Harrington for Craton core" | 19:30 |
sulo | jimbaker: yeah i am ok with merging that, we should implement sigmavirus's suggestion in a separate patch anyway | 19:31 |
jimbaker | sulo, ok, i think that helps us move forward. i'm especially interested in anything that makes say outstanding work we are doing on demos more robust | 19:32 |
jimbaker | need to do the same for the vars stuff, time critical | 19:32 |
sulo | ok one more quick one: https://review.openstack.org/#/c/427879 | 19:47 |
jimbaker | sulo, thanks | 19:50 |
jimbaker | ok, i will go through those | 19:50 |
jimbaker | tojuvone, others - interesting question that just came up on one of the openstack ML: [Openstack-operators] query NUMA topology via API | 19:56 |
jimbaker | this is exactly the sort of thing we could help support | 19:56 |
*** rainya_ has quit IRC | 20:00 | |
*** VW has joined #craton | 20:02 | |
*** jovon has quit IRC | 20:10 | |
sulo | alright, ianyone up to fixing this for demo tomorrow ? | 20:24 |
sulo | https://bugs.launchpad.net/craton/+bug/1661087 | 20:24 |
openstack | Launchpad bug 1661087 in craton "Data generation is generating wrong network address" [Undecided,New] | 20:24 |
*** rainya has joined #craton | 20:26 | |
*** rainya has quit IRC | 20:26 | |
*** kencjohnston has left #craton | 20:35 | |
jimbaker | sulo, i will take a look at that problem | 20:36 |
*** harlowja has joined #craton | 20:39 | |
*** valw has joined #craton | 20:55 | |
*** valw has quit IRC | 21:00 | |
*** Syed__ has quit IRC | 21:05 | |
jimbaker | sulo, please see my comments on https://review.openstack.org/#/c/427717/ | 21:20 |
*** VW has quit IRC | 21:35 | |
*** VW has joined #craton | 21:40 | |
thomasem | jimbaker: there wouldn't have been any objections from me. :) | 21:44 |
*** VW has quit IRC | 21:49 | |
*** VW has joined #craton | 21:54 | |
*** Syed__ has joined #craton | 22:41 | |
*** VW has quit IRC | 23:16 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!