Page MenuHomePhabricator

Review SQL/Python for Phlogiston
Closed, ResolvedPublic

Description

  • Anything in the code that would affect the integrity of the data and interpretation of the results
  • Making it easier to extend reporting to more teams and to maintain reporting
  • General best practices

Code is at https://github.com/wikimedia/phab_task_history

Event Timeline

JAufrecht assigned this task to ksmith.
JAufrecht raised the priority of this task from to Medium.
JAufrecht updated the task description. (Show Details)
JAufrecht updated the task description. (Show Details)
JAufrecht set Security to None.
JAufrecht added subscribers: JAufrecht, Aklapper.

I reviewed the python and gave Joel a bunch of notes. tl;dr was that everything basically looked good, with one potential big speedup (or not), and some ideas for making the code easier to understand.

Any objection to my moving the feedback from a google doc to a new Phab task (so I don't lose it/can treat it as backlog?)

@JAufrecht: Bringing the notes into phab makes sense to me. Go for it.

My SQL skills are "intermediate", so I don't feel capable of doing a thorough review. I will say that the code is well-commented, and all the parts that I did understand (more than half) made perfect sense.

Personally, I would like to see most if not all of the code moved into python. The calls to export tables as CSV would (mostly?) be one-liners, and the munging could be there as well. In some cases, the python would be clearer, and in other cases it would be less clear, but overall I would call it a net win to no longer have raw SQL scripts as part of the process.

ksmith moved this task from In Progress to Needs Review on the Team-Practices (This-Week) board.
ksmith subscribed.

I think my part is done. Feel free to get back to me with questions or to request further review of specific pieces.

JAufrecht renamed this task from Review SQL/Python for Phab burnup reporting script to Review SQL/Python for Phlogiston.Sep 3 2015, 9:05 PM

Done as soon as Joel stores Kevin's email feedback in a permanent, public location. Actual work to address anything is new tasks.