{"action":"created","issue":{"url":"https://api.github.com/repos/taskcluster/taskcluster-client.py/issues/45","repository_url":"https://api.github.com/repos/taskcluster/taskcluster-client.py","labels_url":"https://api.github.com/repos/taskcluster/taskcluster-client.py/issues/45/labels{/name}","comments_url":"https://api.github.com/repos/taskcluster/taskcluster-client.py/issues/45/comments","events_url":"https://api.github.com/repos/taskcluster/taskcluster-client.py/issues/45/events","html_url":"https://github.com/taskcluster/taskcluster-client.py/pull/45","id":138111295,"number":45,"title":"generate code for taskcluster.sync.*","user":{"login":"escapewindow","id":826343,"avatar_url":"https://avatars.githubusercontent.com/u/826343?v=3","gravatar_id":"","url":"https://api.github.com/users/escapewindow","html_url":"https://github.com/escapewindow","followers_url":"https://api.github.com/users/escapewindow/followers","following_url":"https://api.github.com/users/escapewindow/following{/other_user}","gists_url":"https://api.github.com/users/escapewindow/gists{/gist_id}","starred_url":"https://api.github.com/users/escapewindow/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/escapewindow/subscriptions","organizations_url":"https://api.github.com/users/escapewindow/orgs","repos_url":"https://api.github.com/users/escapewindow/repos","events_url":"https://api.github.com/users/escapewindow/events{/privacy}","received_events_url":"https://api.github.com/users/escapewindow/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":11,"created_at":"2016-03-03T08:32:44Z","updated_at":"2016-03-08T20:40:30Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/taskcluster/taskcluster-client.py/pulls/45","html_url":"https://github.com/taskcluster/taskcluster-client.py/pull/45","diff_url":"https://github.com/taskcluster/taskcluster-client.py/pull/45.diff","patch_url":"https://github.com/taskcluster/taskcluster-client.py/pull/45.patch"},"body":"Hey @jhford,\r\n\r\nThis is a pretty big PR, so I don't expect a super fast turnaround.\r\nHowever, the sooner you can alert me to big problems, the less I'll have to rework my upcoming async work.  If you can let me know your ETA/progress, that would be great, thank you!\r\n\r\n### High Level Overview\r\n\r\nWe now generate the api code in `taskcluster.sync` via `make gencode`.  These files are committed to git, the same as apis.json.  That allows us to inspect the files and see any changes easily.\r\n\r\n`taskcluster.client` now imports all the api objects from `taskcluster.sync`.  The existing tests have been modified to allow for this.  New generated code tests check for the existence of all of the appropriate objects and files, and make sure the calls to `_makeHttpRequest` and `_makeTopicExchange` are sane.  Each generated method is limited to 2-4 lines of python code.  The generated code files have 100% test coverage and pass flake8.\r\n\r\nBecause the code and apis.json are tied more closely now, we'll need to run `make update` more often to use the most recent api.  Eventually I'd like to see an automated update / test / dot release whenever the api definitions change.\r\n\r\nI named it `taskcluster.sync` rather than `taskcluster.generated` because I'm planning on creating `taskcluster.async` soon.  The `taskcluster.Queue` and `taskcluster.client.Queue` etc. objects will be sync or async depending on your version of python (due to the imports + `__all__` hacking in `taskcluster/__init__.py` and `taskcluster/client.py`), but if you want to explicitly use the sync or async objects, you can use `taskcluster.sync.*` or `taskcluster.async.*`.\r\n\r\n### Nitty Gritty Details\r\n\r\n* `Makefile` changes:\r\n * I split out `make test` into `make nosetests` and `make lint`.  Now you can test linting or nosetests in any order, although `make test` runs nosetests first.\r\n * Now that the Makefile runs nosetests, coverage, and linting, there's no real reason to keep `test.sh` around.  I nuked it.\r\n * I added a `gencode` target, and made it part of `make update`.\r\n\r\n* `devDep.py` and `setup.py` changes:\r\n * I added a `gencode_requires` list for jinja2.\r\n\r\n* `genCode.Py` - new file that generates the code and test files.\r\n * I named it `genCode.py` to match `genDocs.py`\r\n * Let me know if you need more comments/docs on this\r\n\r\n* `baseclient.py`\r\n * I split out `BaseClient` into `taskcluster.baseclient`, because:\r\n  * the generated code needs to import `BaseClient`\r\n  * `taskcluster.client` imports the generated code\r\n  * if `BaseClient` is in `taskcluster.client`, that creates a circular import dependency chain that barfs\r\n\r\n* `client.py`\r\n * There is now a `RuntimeClient`.\r\n * Essentially, if I used `_makeApiCall` in the generated code, that required me to pass around a full copy of the api definition.  The Releng team also prefers having explicit arguments in each method, rather than `*args` and `**kwargs` everywhere.  So `BaseClient` now has the base logic, and `RuntimeClient` can take the api definition and translate it to `BaseClient`.\r\n * Nothing uses `RuntimeClient` now, but I remember you had a requirement to allow for `createApiClient()` calls at runtime, and keeping it around allows for that.\r\n * We import all api classes from `taskcluster.sync`, and fall through to `createApiClient()` if it doesn't exist.  However, if any don't exist, the `test_generated.py` tests will fail.\r\n\r\n* `taskcluster/sync/[A-Z]*.py`\r\n * These are all generated, so feel free to use whatever level of review scrutiny you think is appropriate for these.\r\n * The `routingKeys` only copies a subset of the routingKeys.  This is largely for file size and line length considerations.  We only use a subset of the keys, so I included the ones we use.  When we use more, we can add them to the `code.template` and `test/base/__init__.py` whitelists.\r\n\r\n* `templates/*.template`\r\n * `code.template` is for the `taskcluster.sync` files other than `__init__.py`, and\r\n * `test.template` is for the generated test files other than `test_generated.py`.\r\n\r\n* `test/base/__init__.py`\r\n * I added support for the generated code tests here.\r\n * `FakeGenerated` is intended for multiple inheritance, e.g. `class FakeAuth(FakeGenerated, Auth):`\r\n  * It replaces `_makeHttpRequest` and `_makeTopicExchange` with `MagicMock()` objects\r\n  * `buildSignedUrl` returns `url + 'SIGNED'`\r\n  * Essentially, this allows for testing code paths and args without testing the underlying logic.\r\n * `GeneratedTC` has generic methods to test topic exchanges and signed/unsigned functions.  It also tests `self.urls` and `self.routingKeys` against apis.json.\r\n * This file could have shared more code with `genCode.py`, but I wanted to avoid the case where we use a broken function to test itself.\r\n\r\n* `test/test[A-Z]*.py`\r\n * These are the generated test files, one per generated code file.\r\n\r\n* `test/test_client.py`\r\n * This required a few changes to account for the `RuntimeClient` / `BaseClient` split.\r\n * If you open `htmlcov/index.html` after running `make test`, you'll see all the generated files have 100% coverage.\r\n\r\n* `test/test_generated.py`\r\n * This uses `apis.json` to make sure all apis have generated classes in `taskcluster.sync`, all apis have files in `taskcluster/sync/`, and no extra .py files are in `taskcluster/sync/`.\r\n"},"comment":{"url":"https://api.github.com/repos/taskcluster/taskcluster-client.py/issues/comments/193958279","html_url":"https://github.com/taskcluster/taskcluster-client.py/pull/45#issuecomment-193958279","issue_url":"https://api.github.com/repos/taskcluster/taskcluster-client.py/issues/45","id":193958279,"user":{"login":"djmitche","id":28673,"avatar_url":"https://avatars.githubusercontent.com/u/28673?v=3","gravatar_id":"","url":"https://api.github.com/users/djmitche","html_url":"https://github.com/djmitche","followers_url":"https://api.github.com/users/djmitche/followers","following_url":"https://api.github.com/users/djmitche/following{/other_user}","gists_url":"https://api.github.com/users/djmitche/gists{/gist_id}","starred_url":"https://api.github.com/users/djmitche/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/djmitche/subscriptions","organizations_url":"https://api.github.com/users/djmitche/orgs","repos_url":"https://api.github.com/users/djmitche/repos","events_url":"https://api.github.com/users/djmitche/events{/privacy}","received_events_url":"https://api.github.com/users/djmitche/received_events","type":"User","site_admin":false},"created_at":"2016-03-08T20:40:30Z","updated_at":"2016-03-08T20:40:30Z","body":"> However, that would lead me to say, we should all use taskcluster.sync.* or taskcluster.async.* rather than importing taskcluster.sync.* into the taskcluster.client and taskcluster namespaces.\r\n\r\nThat sounds OK to me."}}