Wednesday, 2017-02-01

*** rainya_ has quit IRC00:14
*** VW has joined #craton00:26
*** VW has quit IRC00:26
*** VW has joined #craton00:27
*** rainya has joined #craton00:54
*** Syed__ has quit IRC01:25
*** rainya has quit IRC01:41
*** VW has quit IRC01:44
*** git-harry has quit IRC01:49
*** git-harry has joined #craton01:51
*** VW has joined #craton02:09
jimbakerthe 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 #craton03:35
*** Bluebell_ has left #craton03:58
tojuvoneMorning04:17
*** VW has quit IRC04:18
*** VW has joined #craton04:20
*** VW has quit IRC04:38
*** VW has joined #craton04:38
*** VW has quit IRC04:43
*** VW has joined #craton07:39
*** VW has quit IRC07:43
sulohttps://bugs.launchpad.net/craton/+bug/166092209:24
openstackLaunchpad bug 1660922 in craton "Resource search by variable is broken" [High,New] - Assigned to Sulochan Acharya (sulochan-acharya)09:24
git-harrysulo: two of the functional tests are failing for me, is that a regression or were they failing when committed?10:40
sulooh10:40
sulonot sure .. nothign was failing afaik10:40
sulodo you know which ones ?10:41
suloactaully let me try it out10:41
git-harrycraton.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 me10:41
suloah10:42
sulothats probably something else10:42
sulohttps://review.openstack.org/#/c/408784/ not merged yet10:42
suloi need to fix that commit10:42
git-harryWere they broken by changes to the test setup?10:43
suloyeah, thats what it looks like .. all the patches were in chain so its probably that10:44
git-harryWhat is required to get the functional testing enabled in the gate? Is there any reason it's not already enabled?10:45
sulogit-harry: i think we should enable it .. just that no one done it yet10:45
sulogit-harry: also, we should enable coverage gen and look at whats missing10:46
sulofound a big bug on afeature we needed because things changed and we didnt10:46
sulohave enough tests to cover that condition10:46
git-harrysounds good10:47
git-harrygetting the stuff enabled that is, not the bug.10:47
sulojimbaker: sigmavirus: git-harry please check openstack-dev ml for my email and +/- accordingly, thx11:09
*** odyssey4me has left #craton11:17
*** VW has joined #craton13:08
*** VW has quit IRC13:08
*** VW has joined #craton13:08
*** VW has quit IRC13:09
*** VW has joined #craton13:10
*** Mitschke has quit IRC13:11
*** VW has quit IRC13:14
*** Mitschke has joined #craton13:49
*** valw has joined #craton13:52
*** VW has joined #craton13:55
sulohttps://review.openstack.org/#/c/427717/ << vars filtering14:08
*** valw has quit IRC14:12
*** valw has joined #craton14:13
sigmavirussulo: tests are failing14:16
sulofixed14:18
*** valw has quit IRC14:36
thomasemo/14:42
thomasemsulo: in master or in review?14:45
sulothomasem: review15:06
thomasemgotcha15:07
thomasemWanted to be sure I can still expect these two test failures in master, since I got them this morning. :D15:07
thomasemThanks!15:07
*** sigmavirus has quit IRC15:14
sulothomasem: for functional tests ? if so there is pr up to fix that.15:26
*** sigmavirus has joined #craton15:26
*** ChanServ sets mode: +o sigmavirus15:26
suloactually, that needs to be updated15:26
sulolet me do that now15:26
thomasemsulo: yes, that's why I was asking. I think that patch needs a rebase.15:26
suloyeah, fixing that now15:26
thomasemWoot15:26
thomasemThanks sulo15:26
*** jovon has joined #craton15:30
sulook rebased and updated https://review.openstack.org/#/c/408784/15:43
thomasemsulo: reviewing15:44
sulothanks, afk for a bit15:44
jimbakersulo, ack about your email15:56
*** VW has quit IRC15:56
*** VW has joined #craton15:57
jimbakersulo, thanks for the fixes on functional testing of regions. nice to see tox -e functional is working without problems again with this change16:01
jimbakernext: we can make our functional tests stronger. but a future change!16:01
jimbakersulo, 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/40878416:04
jimbakersigmavirus, 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-lookup16:15
jimbakermaybe a better doc somewhere else?16:15
sigmavirusjimbaker: so if you do `from requests import status_codes as status; status.OK`16:15
sigmaviruser16:15
jimbakerfor 422, https://github.com/kennethreitz/requests/blob/master/requests/status_codes.py#L5716:15
sigmavirusstatus.codes.OK16:15
sigmavirusthat'll work16:16
sigmavirusstatus.codes.unprocessable16:16
sigmavirusetc.16:16
sigmavirusthat will return 42216:16
sigmavirusbut it's better not to write 422 explicitly IMO16:16
sigmavirusthe code name generally provides a good explanation in the test16:16
jimbakersure, 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 autocomplete16:18
jimbakeralthough the docs actually reference `requests.codes`, which doesn't autocomplete16:19
jimbakerfor some reason16:19
jimbakermaybe some additional magic?16:19
jimbakeranyway, i will attach my findings to the review16:20
sigmavirusWe do import the codes object into the requests namespace https://github.com/kennethreitz/requests/blob/master/requests/__init__.py#L6716:20
sigmavirussulo: I'll be happy to update your review16:20
sigmavirusI'm kind of hoping for the functional bases to be present before continuing on pagination work16:21
jimbakersounds good16:21
tojuvoneHi guys16:22
sigmavirusHey tojuvone16:23
tojuvoneWas thinking if get Nova to link Craton, it would be totally cool for the project16:23
jimbakeralso 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 file16:23
tojuvonetherefor tried to think a bit the namespace thing16:23
tojuvonePut some here, what I got by fast: http://pastebin.com/NM33cfXu16:24
jimbakertojuvone, thanks. re setting variables in one call - yes, this is implemented atomically as a single db transaction16:25
jimbakerwe have to ensure that our notification process actually respects that of course, and doesn't send out notifications on a per row basis16:26
tojuvoneyes, would think notification payload would hold normally what in that namespace16:27
*** Syed__ has joined #craton16:27
jimbakerbut i'm pretty sure right now that our rest apis consistently follow a unit of work - one rest api call is one db transaction16:27
tojuvonethen wondering how the hostid thing would work16:28
tojuvoneboth admin and other user might not be allowed to get same variables16:29
jimbakerso this is the first time we have discussed going from nova to craton to look up info, so that's an interesting twist16:30
jimbakerbut 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
tojuvonewell, I had this discussion before to some external thing owning this palnned host maintenance16:31
jimbakermore importantly, the host id we use is opaque, so nova could store it if desired16:32
tojuvoneNow it would be Craton.16:32
jimbakerright16:32
jimbakerthat's definitely something we would store - this is what we did with the similar inventory system in rackspace public cloud16:33
tojuvoneTought that this might be in away now quite fance thing to figure out to people to start using Craton16:33
tojuvoneNova do not want any extra data (been there). Meaning I would not expect anything more than what I put there16:35
tojuvoneI mean to store some "hostid" coming from Craton16:35
jimbakerright, so we are definitely focused on storing this type of data. and doing further elaboration as we discussed about a set of maintenance variables16:35
tojuvoneAnyhow that hostid is already known by Nova16:36
sigmavirusjimbaker: it's expected, value16:36
jimbakersigmavirus, thanks for the clarification :)16:37
*** valw has joined #craton16:37
jimbakerbut first, same file consistency. even better, general consistency16:37
*** valw has quit IRC16:41
jimbakersigmavirus, 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
jimbakerin other words, this specific assertion is verifying that we got back two regions, but it's quite cryptic in its expectations16:45
sigmavirusjimbaker: Yeah, I'm already working on that in the unit tests16:47
sigmavirusThat said, in the unit tests, where we mock out the dbapi, I feel conflicted16:47
jimbakerwell those specific tests are certainly super weak16:53
jimbakergiven that there's not so much surface to be tested once the dbapi functionality is mocked out16:53
*** VW has quit IRC17:01
sigmavirusjimbaker: right, I'm really not a fan of craton.tests.unit.test_api at all17:01
*** VW has joined #craton17:04
tojuvoneSo thinking when different things might be implemented...17:27
tojuvoneMaybe I should start soon drafting spec to Nova to have it in Pike to have something in time of Sydney17:28
tojuvoneThen just need to know how it can be in Craton side17:29
tojuvoneGood thing is that for Nova I have code quite ready if just get spec trough17:32
sigmavirustojuvone: propose the spec and code simultaneously and asap to allow it to have any chance of landing in Pike.17:34
sigmavirusI would guess, that you'll get it in during Queens at the earliest though17:34
sigmavirusI think Nova already has several goals set/specs set for Pike already17:34
*** valw has joined #craton17:38
*** rainya has joined #craton17:41
*** valw has quit IRC17:43
jimbakerright, 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 better17:51
jimbakerbut we will do so by working out the spec17:51
jimbakerand corresponding code17:52
jimbakertojuvone, ^^^17:52
tojuvoneSorry, was doing dishes :D17:56
tojuvoneYes, luckily not very big change, but not the easiest either17:57
Syed__jimbaker: a question18:04
Syed__so i installed cratonclient on top of craton and trying to query some simple commands18:05
Syed__https://www.irccloud.com/pastebin/4s2vrvOl/18:05
jimbakerSyed__, i assume you have set your env vars18:05
*** harlowja_ has joined #craton18:05
jimbakerbut if not, here's what i set18:05
Syed__hmm, its a local install, not through docker18:06
*** harlowja has quit IRC18: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 setup18:07
sulojimbaker: sigmavirus: just got back, reading scrollback18:07
jimbakerSyed__, https://gist.github.com/jimbaker/559dd5c9d8933ea94a7e3798429fafcd18:07
jimbakerSyed__, the Dockerfile instructions are precise18:08
jimbakerbut here's the script i use for starting and populating a craton instance with some fake data to play with18:08
jimbakerhttps://gist.github.com/jimbaker/e74a7b98bc60519033fd455a22163ad218:08
Syed__got it, cool18:09
Syed__thanks18:09
jimbakeronce that is finished running, you should just be able to run the cli against it, without problems. it just works18:09
jimbakeralso 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
jimbakeri have other steps that i run if i want to run the db more directly18:11
thomasemYeah. 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 method18:12
sulojimbaker: sigmavirus: so instead of 200, status_codes.code.ok for example ?18:13
sigmavirusexactly sulo18:13
thomasemMy 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
sigmavirusIf you want I can touch it up, assuming you're off for the rest of the day18:13
suloyeah i am around but not active right now18:15
sulosigmavirus: actually, i am a little confused with this, what do we gain from doing that ?18:17
thomasemMore reader-friendly code18:18
sigmavirussulo: having hard-coded status numbers in there is really gross18:19
thomasemMore idiomatic implementation18:19
sigmavirusthey end up being magic-ish numbers18:19
sulobut they are status code .. the whole wide world uses them?18:19
suloi dont mind chaging it though18:19
jimbakerthomasem, yeah, we definitely want to get to something that is production18:20
thomasemjimbaker: we shall :)18:20
jimbakersome fairly obvious approaches, but i think we can do it even better with a little containerization18:20
sulosigmavirus: thomasem: https://github.com/kennethreitz/requests/blob/f72684e13c5074a671506d29c1b5638156680ea7/tests/test_requests.py#L16018:21
thomasemCertainly has my vote.18:21
sigmavirussulo: if requests jumped off a bridge, would you do the same?18:21
sulosigmavirus: you are aksing to do this because of requests as per your comment18:21
thomasemsulo: I want to point out, I personally have no problem using the hard-coded status code. It's a stylistic thing.18:21
sulosigmavirus: i am making a case that i dont see the point in doing this at all18:22
sigmavirussulo: no, I never said "because of requests"18:22
*** VW has quit IRC18:22
sulosigmavirus: "We shouldn't hardcode numbers here for status codes. Requests has a way of referencing them by name"18:22
*** VW has joined #craton18:22
sigmavirusYes, that's not "because requests does it" that's because it's convenient and easier to read to use the reason name18:22
suloanyhow, i dont see the point18:23
suloyall do the reviews, ill chage it if thats the vote18:23
thomasemIn 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-harrysulo: 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
thomasemOtherwise some reviews are going to get pinged for it, and others won't.18:24
thomasemThere's no controls around it.18:24
thomasemAll that said, we're taking more time talking about it than it takes to make the change now.18:25
thomasemHmmm, weird... doesn't seem to be picking up my schemas.py changes18:27
sigmavirusthomasem: ?18:28
thomasemI'm adding "projects_id_variables" and getting 500 when I try to talk to it for some reason.18:29
sigmavirusthomasem: check craton-api.log18:29
thomasemYerp, digging in18:29
thomasemThanks18:29
sigmavirushappy to hop onto a vidyo call if you need help18:29
thomasemsigmavirus: I appreciate it! I'll letcha know if I have trouble figuring it out. :)18:30
*** rainya_ has joined #craton18:37
*** rainya has quit IRC18:40
jimbakeri agree with sulo and thomasem - i was ready to approve the change https://review.openstack.org/#/c/408784/ because it actually fixes a broken build19:13
jimbakersomehow we got ourselves into this with the recent set of changes, and a few mixups19:13
jimbakerbut without sulo's change, tox -e functional fails, and we have to know that it fails and workaround that in looking at things19:13
jimbakerso 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 documented19:14
jimbaker(without reference to source code; the usage examples are not comprehensive)19:15
jimbakerthomasem, 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 IRC19:18
*** harlowja_ has quit IRC19:19
jimbakerok, added my 2ยข to https://review.openstack.org/#/c/408784/19:25
jimbakersigmavirus, sulo, thomasem, any objections for me +2-ing as well?19:26
jimbakergit-harry, sorry, you were also in the conversation19:29
jimbakersigmavirus, also separately, please respond to sulo's email "[openstack-dev] [craton] Proposing Harry Harrington for Craton core"19:30
sulojimbaker: yeah i am ok with merging that, we should implement sigmavirus's suggestion in a separate patch anyway19:31
jimbakersulo, 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 robust19:32
jimbakerneed to do the same for the vars stuff, time critical19:32
sulook one more quick one: https://review.openstack.org/#/c/42787919:47
jimbakersulo, thanks19:50
jimbakerok, i will go through those19:50
jimbakertojuvone, others - interesting question that just came up on one of the openstack ML: [Openstack-operators] query NUMA topology via API19:56
jimbakerthis is exactly the sort of thing we could help support19:56
*** rainya_ has quit IRC20:00
*** VW has joined #craton20:02
*** jovon has quit IRC20:10
suloalright, ianyone up to fixing this for demo tomorrow ?20:24
sulohttps://bugs.launchpad.net/craton/+bug/166108720:24
openstackLaunchpad bug 1661087 in craton "Data generation is generating wrong network address" [Undecided,New]20:24
*** rainya has joined #craton20:26
*** rainya has quit IRC20:26
*** kencjohnston has left #craton20:35
jimbakersulo, i will take a look at that problem20:36
*** harlowja has joined #craton20:39
*** valw has joined #craton20:55
*** valw has quit IRC21:00
*** Syed__ has quit IRC21:05
jimbakersulo, please see my comments on https://review.openstack.org/#/c/427717/21:20
*** VW has quit IRC21:35
*** VW has joined #craton21:40
thomasemjimbaker: there wouldn't have been any objections from me. :)21:44
*** VW has quit IRC21:49
*** VW has joined #craton21:54
*** Syed__ has joined #craton22:41
*** VW has quit IRC23:16

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