mozilla :: #conduit

20 Mar 2017
13:29imaduemesmacleod: Any thoughts on https://reviewboard.mozilla.org/r/121056/ ?
13:30smacleodYa i was mid rrview. Going into my appt thougg so it will have to wait
13:30smacleodimadueme: ^
13:31imaduemeAh no problem, thanks smacleod
13:38dklmorning
13:39* imadueme
14:36smacleodmars: FYI, I gave the commitindex web api swagger review stuff to dkl - I switched away from flask-restplus to connexion, which seems *much* better to me.
14:36dklsmacleod: yeah thanks. going to look at that today
14:40imaduemealso smacleod: is this available https://trello.com/c/9y3hoYOM/168-1-create-an-openapi-specification-for-the-staging-repo-s-iteration-proxy-end-point somewhere?
14:41smacleodimadueme: nope, I think you should take that card
14:41smacleodI never got around to writing it
14:41smacleodYou should be able to bang it out in an hour or so, it'll be good for learning the swagger stuff too
14:41imaduemeOk cool, I'll grab it
14:42smacleodimadueme: you can take a look at the swagger.yml for the commit index here https://reviewboard.mozilla.org/r/121758/diff/1#index_header as an example
14:44imaduemesmacleod: great. and to confirm there is no tool i need to use to actually write it. I manually write the yml and the tool (connexion) maps the spec to our code (I'm guessing we're not using it for the hg server tho).
14:44imadueme?
14:45smacleodya connexion won't be used for the hg one, this is more just so we have a spec defined in this case.
14:45smacleodimadueme: http://swagger.io/swagger-editor/
14:45smacleodthat'll help write it ^
14:45imaduemeawesome
14:46zalunmars: sococo? empty vidyo room ...
14:46marszalun: vidyo, omw
16:54davidwalshmars: So I'm running through our demo but suddenly I'm not seeing the 400 from imadueme's work when I push; do you recall if there was another demo step we completed?
16:56marsdavidwalsh: I think you have to run conduitstage? the new command should be in 'hg help commands' I think
16:58davidwalshAhh yes, thanks
17:48marsdavidwalsh: review published, just minor stuff. I can land it once the changes are done
17:50davidwalshCool
17:56davidwalshmars: None of our Dockerfile's have MPL; should we stay consistent and leave off for now?
17:56marssmacleod: dkl: passing around a logger object is actually an OO anti-pattern in Python (I think it came from Java). The "log = logging.getLogger(__name__)" pattern is the right way to do it, and has advantages with shorter code and produces better log messages.
17:58marsdavidwalsh: oh, OK. I thought I saw it in other files. It's fine to leave it out then.
17:59smacleodmars: I was talking about the name only, not passing sound
17:59smacleodAround*
17:59smacleodSo the pattern you suggest, but calling it logger, rather than log
17:59marsI always saw the shorter "log.info()" when I looked at Python libs
18:00smacleodYou haven't looked at all libs, or the same ones as me then
18:00marsless typing, it's what I learned and saw? *shrugs*
18:01smacleodVct uses "logger" and so does autoland, might as well stay consistent
18:01marsI meant not our libs here, I meant the code we wrote in other Python projects and third-party code
18:03smacleodI meant both
18:04dklmars: if i write 'log = logging.getLogger(__name__)' at the top of the module I get ' Invalid constant name "log"'
18:04dklmars: if I do 'LOGGER = ...' it is fine
18:05marshmm, python 3 change?
18:05dklthink so
18:05dklnoticing little quirks here and there
18:05dklhad to get used to doing print()
18:06marssmacleod: dkl, I see the python 3 docs themselves say "logger.blah()"
18:06marsoops, "logging.blah()"
18:08marsdkl, the logging docs themselves say that "logger = logging.getLogger(__name__)" should work?
18:08marshttps://docs.python.org/3/howto/logging.html#advanced-logging-tutorial
18:09dklmars: which i am sure it does, just doesnt like it at the top of the module and wants it wrapped in a Class or main()
18:10marsdkl: what is giving you that error? I just tried it with a trivial 3-line file and the python3.5 interpreter, and it ran without error.
18:11dklmars. it runs fine. pylint doesnt like it
18:11dkli was trying to clean out any errors from pylint before submitting for review
18:11dklis there a different pylint for python2 and 3?
18:12marsah! that's pylint's fault then. We should switch that off for that variable, if we can
18:12marsdkl: might be pylint not special-casing module-level vars named 'logger'
18:13dklmars: i get the same warning whether it is 'log' or 'logger'
18:13marsdkl: this should work around it: http://stackoverflow.com/a/1885262
18:13smacleoddkl why are you running "pylint"?
18:14dklbasically fails on anything not all upper case
18:14smacleodUse invoke lint
18:14smacleodOur invoke tasks run the linting we care about, flake8, not pylint
18:16marswell, I don't see why we can't use pylint in addition to that, but it's not the official project linter we need to pass to land code :)
18:16dklsmacleod: i was just running it locally just to help me catch careless newbie errors
18:16dklmars: ok. so i will add exceptions to my local pylint config
18:16smacleoddkl: ya, run flake8 for that, not pylint
18:17marsdkl: ^ yes, switch your editor over if you can
18:17dklok
18:29davidwalshmars: Fixed
18:31mcotey'all seem to be kicking butt on this sprint :D
18:52pulsebotCheck-in: https://hg.mozilla.org/automation/conduit/rev/c462aef96eb2 - David Walsh - Conduit: Create the initial conduit demo (Bug 1348880). r=mars
18:58marsdavidwalsh: ^
18:58davidwalsh\o/
19:01circleci-botSuccess: davehouse's build (#53; push) in mozilla-conduit/conduit (master) -- https://circleci.com/gh/mozilla-conduit/conduit/53?utm_campaign=chatroom-integration&utm_medium=referral&utm_source=irc
20:05marssmacleod: is the iterations endpoint card in review?
20:11marsnm, found it
20:14smacleodmars: woops, was meant to be in progress, heh
20:17marssmacleod: so will this code respond to the /iterations/ endpoint too?
20:18marsI'm wondering if the api package module names are magic
20:26marslooks like they are, and the endpoint should just work. nice.
20:28dklhmm. getting a hang when pushing up revised commits to reviewboard https://dkl.pastebin.mozilla.org/8982673
20:28dkljust sits there. if i cancel and try again. same
20:42marsoh, I wonder if that was the same bug that the banner in mozreview was about?
20:44mcotedkl: how long did you wait?
20:44mcotemars: I don't think so, as it sounds like dkl's review requests do end up getting published
20:44dklmcote: 2-3 mins
20:44mcoteyeah I think the 500 errors time out faster than that
20:46marssmacleod: I split up the 13 point user story: https://public.etherpad-mozilla.org/p/ARzHPFxYTv
20:48marssmacleod: LMK how that split looks - I'm happy to go over it in a quick voice call
20:50smacleodmars: the are a few issues, can't voice chat now though. I'll look more in depth soonish
20:50marssmacleod: I don't think we need a story for duplicate POSTs if the behaviour is to always create a new iteration
20:51smacleodmars: I agree, I thought it'd be more clear though if I wrote it that way first instead of just deleting your work
21:17dklsmacleod: hmm i do not see from the reviewboard UI where to r+ I can add comments, etc. but not r+
21:17dkldo i need more perms? I am logged in as myself
23:22imadueme|bbldkl: There should be a "finish review" green button
21 Mar 2017
No messages
   
Last message: 5 days and 22 hours ago