Fully functioning Balrog Agent

[bhearsum]

(I don't suggest looking at this pull request to review. This is built on top of a series of patches, so the PR includes a bunch of stuff that's not really part of the agent. I think the best view of it is https://github.com/mozbhearsum/balrog/compare/rule-changes-web...balrog-agent?expand=1.)

I've gone through a cleanup pass on this, but I think it can probably be improved more still. Here's the highlights:

• Contains all the scaffolding to do CI/docker image builds/etc. This is mostly copied from the top level directory, with a few necessary tweaks.
• client.py started off as a Python 3 implementation of api.py, but I quickly found that it made more sense from a clean architecture standpoint to rip out a lot of those guts. It's so small now I'm not even sure it's worth keeping....it really just handles url creation and calling raise_for_status().
• changes.py is meant to be where the core business logic is handled, so far this is just is_ready(). I explicitly chose to pass in time and current_uptake to keep this a pure function. Doing this means we need repeated if blocks in here+run_agent though...which is annoying.
• run_agent feels messy to me right now, and isn't very testable. Maybe that's okay because it's mostly glue code?

@rail - I'd love to get your thoughts on this, you have pretty good asyncio/aiohttp knowledge.

[bhearsum]

@rail - What do you think of the current state of things? I've fixed up everything you mentioned. The test coverage is pretty bad, but client.py and cmd.py are not very testable :(.

[escapewindow]

I did things like https://github.com/escapewindow/scriptworker/blob/master/scriptworker/poll.py#L69 :

If you wanted to make request() more testable, you could do request_function=aiohttp.request here, and then call await request_function(...). In the tests you can pass a dummy request function that can return whatever values you want. To hit the second raise_for_status() you could have a counter that adds an exception to the response the 2nd time it's called.

I also ended up mocking the heck out of everything to get 100% unittest coverage for scriptworker.worker: https://github.com/escapewindow/scriptworker/blob/master/tests/test_worker.py#L107

and I added integration tests to make sure it actually worked against taskcluster: https://github.com/escapewindow/scriptworker/blob/master/tests/test_integration.py

So testing these things is doable. It takes effort. I think it's worth it in avoiding future bugs and mistakes. Also, it's easier to r- when someone submits a PR without tests if you're at 100% coverage and their patch drops it to 99%, rather than if someone submits a PR and drops your coverage from 49% to 48%, for example.

[bhearsum]

Alright, this latest push adds tests for the vast majority of the business logic. changes.py is fully tested (minus get_telemetry_uptake, which is just a stub). run_agent is mostly tested, except for the parts that make it an infinite loop in production.

[bhearsum]

@escapewindow or @rail - any additional thoughts on this?

[escapewindow]

I think I'm good. Thanks for being open minded about tests!

[rail]

Could have use logging.error(msg, exc_info=True) to avoid extra imports.

[rail]

Not critical though.

[bhearsum]

OK @rail, I think this is ready for a final look now.

[rail]

🚜 it