{"action":"created","comment":{"url":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/comments/25202115","id":25202115,"diff_hunk":"@@ -21,93 +21,80 @@ class ApiDeclarationNotFoundError(Exception):\n \n \n def find_resource_names(api_docs_json):\n-    return [\n-        api['path'].lstrip('/')\n-        for api in api_docs_json['apis']\n-    ]\n+    return [api['path'].lstrip('/') for api in api_docs_json['apis']]\n \n \n-def build_schema_mapping(schema_dir):\n+def build_schema_mapping(schema_dir, listing_json):\n     \"\"\"Discovers schema file locations and relations.\n \n     :param schema_dir: the directory schema files live inside\n     :type schema_dir: string\n-    :returns: A tuple of (resource listing filepath, mapping) where the mapping\n-        is between resource name and file path\n-    :rtype: (string, dict)\n+    :param listing_json: the contents of the listing document\n+    :type  listing_json: string\n+    :returns: a mapping from resource name to file path\n+    :rtype: dict\n     \"\"\"\n     def resource_name_to_filepath(name):\n         return os.path.join(schema_dir, '{0}.json'.format(name))\n \n-    listing, listing_json = _load_resource_listing(schema_dir)\n-\n-    return (\n-        listing,\n-        dict(\n-            (resource, resource_name_to_filepath(resource))\n-            for resource in find_resource_names(listing_json)\n-        )\n+    return dict(\n+        (resource, resource_name_to_filepath(resource))\n+        for resource in find_resource_names(listing_json)\n     )\n \n \n-def _load_resource_listing(schema_dir):\n+def _load_resource_listing(resource_listing):\n     \"\"\"Load the resource listing from file, handling errors.\n \n-    :param schema_dir: the directory schema files live inside\n-    :type schema_dir: string\n-    :returns: (resource listing filepath, resource listing json)\n+    :param resource_listing: path to the api-docs resource listing file\n+    :type  resource_listing: string\n+    :returns: contents of the resource listing file\n+    :rtype: dict\n     \"\"\"\n-    resource_listing = os.path.join(schema_dir, API_DOCS_FILENAME)\n     try:\n         with open(resource_listing) as resource_listing_file:\n-            resource_listing_json = simplejson.load(resource_listing_file)\n+            return simplejson.load(resource_listing_file)\n     # If not found, raise a more user-friendly error.\n     except IOError:\n         raise ResourceListingNotFoundError(\n             'No resource listing found at {0}. Note that your json file '\n             'must be named {1}'.format(resource_listing, API_DOCS_FILENAME)\n         )\n-    return resource_listing, resource_listing_json\n \n \n-def compile_swagger_schema(schema_dir, should_validate_schemas):\n+def compile_swagger_schema(schema_dir):\n     \"\"\"Build a SwaggerSchema from various files.\n \n     :param schema_dir: the directory schema files live inside\n     :type schema_dir: string\n-    :param should_validate_schemas: if True, check schemas for correctness\n-    :type should_validate_schemas: boolean\n     :returns: a SwaggerSchema object\n     \"\"\"\n-    listing, mapping = build_schema_mapping(schema_dir)\n-    schema_resolvers = ingest_resources(\n-        listing,\n-        mapping,\n-        schema_dir,\n-        should_validate_schemas,\n-    )\n-    return SwaggerSchema(\n-        listing,\n-        mapping,\n-        schema_resolvers,\n-    )\n+    listing_filename = os.path.join(schema_dir, API_DOCS_FILENAME)\n+    listing_json = _load_resource_listing(listing_filename)\n+    mapping = build_schema_mapping(schema_dir, listing_json)","path":"pyramid_swagger/ingest.py","position":92,"original_position":92,"commit_id":"473b5d5d1219f15f1d4aa7c2fd5007ccab48d92e","original_commit_id":"473b5d5d1219f15f1d4aa7c2fd5007ccab48d92e","user":{"login":"prat0318","id":1528697,"avatar_url":"https://avatars.githubusercontent.com/u/1528697?v=3","gravatar_id":"","url":"https://api.github.com/users/prat0318","html_url":"https://github.com/prat0318","followers_url":"https://api.github.com/users/prat0318/followers","following_url":"https://api.github.com/users/prat0318/following{/other_user}","gists_url":"https://api.github.com/users/prat0318/gists{/gist_id}","starred_url":"https://api.github.com/users/prat0318/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/prat0318/subscriptions","organizations_url":"https://api.github.com/users/prat0318/orgs","repos_url":"https://api.github.com/users/prat0318/repos","events_url":"https://api.github.com/users/prat0318/events{/privacy}","received_events_url":"https://api.github.com/users/prat0318/received_events","type":"User","site_admin":false},"body":"Do we need to call `build_schema_mapping` again in line 84? I was thinking to instead call the `validate_swagger_schema` from inside the `build_schema_mapping` and pass the `mapping` to `validate_swagger_schema` during the call. This way `validation` will happen before `ingest_resources` and with a single call to `build_schema`.","created_at":"2015-02-23T21:07:04Z","updated_at":"2015-02-23T21:07:04Z","html_url":"https://github.com/striglia/pyramid_swagger/pull/82#discussion_r25202115","pull_request_url":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/82","_links":{"self":{"href":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/comments/25202115"},"html":{"href":"https://github.com/striglia/pyramid_swagger/pull/82#discussion_r25202115"},"pull_request":{"href":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/82"}}},"pull_request":{"url":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/82","id":29670386,"html_url":"https://github.com/striglia/pyramid_swagger/pull/82","diff_url":"https://github.com/striglia/pyramid_swagger/pull/82.diff","patch_url":"https://github.com/striglia/pyramid_swagger/pull/82.patch","issue_url":"https://api.github.com/repos/striglia/pyramid_swagger/issues/82","number":82,"state":"open","locked":false,"title":"Make api docs route optional","user":{"login":"dnephin","id":442180,"avatar_url":"https://avatars.githubusercontent.com/u/442180?v=3","gravatar_id":"","url":"https://api.github.com/users/dnephin","html_url":"https://github.com/dnephin","followers_url":"https://api.github.com/users/dnephin/followers","following_url":"https://api.github.com/users/dnephin/following{/other_user}","gists_url":"https://api.github.com/users/dnephin/gists{/gist_id}","starred_url":"https://api.github.com/users/dnephin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dnephin/subscriptions","organizations_url":"https://api.github.com/users/dnephin/orgs","repos_url":"https://api.github.com/users/dnephin/repos","events_url":"https://api.github.com/users/dnephin/events{/privacy}","received_events_url":"https://api.github.com/users/dnephin/received_events","type":"User","site_admin":false},"body":"I believe this resolves #73\r\n\r\n2b092b1 flatten out the call-graph in `ingest.py`. While I was trying to grok this module I noticed that the call graph was pretty deep, but a lot of parameters weren't being used for anything except being passed down to the next function.  With a couple small changes I was able to get everything called directly from `compile_swagger_schema()` which is the entrypoint to this module.  My feeling is that this makes it easier to understand because from a single function (`compile_swagger_schema()`) you can get a sense of the whole module. The call graph is never really more than depth 2. It also simplies the return value of the functions to single values instead of having to return complex tuples from many of them.\r\n\r\n0a44a58 was just a sed. `register_swagger_endpoints` made me think of #3. In this case \"swagger\" should be pretty obvious since the package is already called `pyramid_swagger`, so I thought `register_api_doc_endpoints` was more accurate.\r\n\r\nc259561 addresses the first point of #73. Adds an option to completely disable /api-docs registration, leaving it up to the user. Docs updated.\r\n\r\n 4c624c3 starts to get more involved in trying to address the second point of #73.  I noticed that `compile_swagger_schema()` (and `validate_swagger_schema()` under that) were being called from two places.  I guess this is because `pyramid_swagger` has two primary functions: serving api docs, and the tween for request/response validation.  Each was calling compile/validate.  It seemed like instead of having them both do the work, we could move that work to be a the first step of including this package.  The compile and validate can be done up-front, and the artifact ( the `SwaggerSchema`) just gets added to the settings. I think we could also make an interface and have this registered to the registry redirectly (let me know if that feels like a better approach, I wasn't exactly sure what the function was to do that).  \r\n\r\n\r\nThis change supports the second point in #73 by allowing a user to disable this `compile_swagger_schema()` step entirely.  This would allow them to construct their own `SwaggerSchema` in whatever method they want, add it to the settings (or registry) and everything else in the package should work the same way. \r\n\r\nThis change still needs some documentation about the `SwaggerSchema` interface, and how you would build one yourself (and possibly more tests), but I wanted to get it out and get some feedback on it before spending more time on it.\r\n\r\n\r\n\r\n","created_at":"2015-02-20T03:13:36Z","updated_at":"2015-02-23T21:07:04Z","closed_at":null,"merged_at":null,"merge_commit_sha":"fe36586dda917ddab4e248fa9f50d1fe9c92305f","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/82/commits","review_comments_url":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/82/comments","review_comment_url":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/comments{/number}","comments_url":"https://api.github.com/repos/striglia/pyramid_swagger/issues/82/comments","statuses_url":"https://api.github.com/repos/striglia/pyramid_swagger/statuses/473b5d5d1219f15f1d4aa7c2fd5007ccab48d92e","head":{"label":"dnephin:make_api_docs_route_optional","ref":"make_api_docs_route_optional","sha":"473b5d5d1219f15f1d4aa7c2fd5007ccab48d92e","user":{"login":"dnephin","id":442180,"avatar_url":"https://avatars.githubusercontent.com/u/442180?v=3","gravatar_id":"","url":"https://api.github.com/users/dnephin","html_url":"https://github.com/dnephin","followers_url":"https://api.github.com/users/dnephin/followers","following_url":"https://api.github.com/users/dnephin/following{/other_user}","gists_url":"https://api.github.com/users/dnephin/gists{/gist_id}","starred_url":"https://api.github.com/users/dnephin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dnephin/subscriptions","organizations_url":"https://api.github.com/users/dnephin/orgs","repos_url":"https://api.github.com/users/dnephin/repos","events_url":"https://api.github.com/users/dnephin/events{/privacy}","received_events_url":"https://api.github.com/users/dnephin/received_events","type":"User","site_admin":false},"repo":{"id":30991354,"name":"pyramid_swagger","full_name":"dnephin/pyramid_swagger","owner":{"login":"dnephin","id":442180,"avatar_url":"https://avatars.githubusercontent.com/u/442180?v=3","gravatar_id":"","url":"https://api.github.com/users/dnephin","html_url":"https://github.com/dnephin","followers_url":"https://api.github.com/users/dnephin/followers","following_url":"https://api.github.com/users/dnephin/following{/other_user}","gists_url":"https://api.github.com/users/dnephin/gists{/gist_id}","starred_url":"https://api.github.com/users/dnephin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dnephin/subscriptions","organizations_url":"https://api.github.com/users/dnephin/orgs","repos_url":"https://api.github.com/users/dnephin/repos","events_url":"https://api.github.com/users/dnephin/events{/privacy}","received_events_url":"https://api.github.com/users/dnephin/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/dnephin/pyramid_swagger","description":"Convenient tools for using Swagger to define and validate your interfaces in a Pyramid webapp.","fork":true,"url":"https://api.github.com/repos/dnephin/pyramid_swagger","forks_url":"https://api.github.com/repos/dnephin/pyramid_swagger/forks","keys_url":"https://api.github.com/repos/dnephin/pyramid_swagger/keys{/key_id}","collaborators_url":"https://api.github.com/repos/dnephin/pyramid_swagger/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/dnephin/pyramid_swagger/teams","hooks_url":"https://api.github.com/repos/dnephin/pyramid_swagger/hooks","issue_events_url":"https://api.github.com/repos/dnephin/pyramid_swagger/issues/events{/number}","events_url":"https://api.github.com/repos/dnephin/pyramid_swagger/events","assignees_url":"https://api.github.com/repos/dnephin/pyramid_swagger/assignees{/user}","branches_url":"https://api.github.com/repos/dnephin/pyramid_swagger/branches{/branch}","tags_url":"https://api.github.com/repos/dnephin/pyramid_swagger/tags","blobs_url":"https://api.github.com/repos/dnephin/pyramid_swagger/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/dnephin/pyramid_swagger/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/dnephin/pyramid_swagger/git/refs{/sha}","trees_url":"https://api.github.com/repos/dnephin/pyramid_swagger/git/trees{/sha}","statuses_url":"https://api.github.com/repos/dnephin/pyramid_swagger/statuses/{sha}","languages_url":"https://api.github.com/repos/dnephin/pyramid_swagger/languages","stargazers_url":"https://api.github.com/repos/dnephin/pyramid_swagger/stargazers","contributors_url":"https://api.github.com/repos/dnephin/pyramid_swagger/contributors","subscribers_url":"https://api.github.com/repos/dnephin/pyramid_swagger/subscribers","subscription_url":"https://api.github.com/repos/dnephin/pyramid_swagger/subscription","commits_url":"https://api.github.com/repos/dnephin/pyramid_swagger/commits{/sha}","git_commits_url":"https://api.github.com/repos/dnephin/pyramid_swagger/git/commits{/sha}","comments_url":"https://api.github.com/repos/dnephin/pyramid_swagger/comments{/number}","issue_comment_url":"https://api.github.com/repos/dnephin/pyramid_swagger/issues/comments{/number}","contents_url":"https://api.github.com/repos/dnephin/pyramid_swagger/contents/{+path}","compare_url":"https://api.github.com/repos/dnephin/pyramid_swagger/compare/{base}...{head}","merges_url":"https://api.github.com/repos/dnephin/pyramid_swagger/merges","archive_url":"https://api.github.com/repos/dnephin/pyramid_swagger/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/dnephin/pyramid_swagger/downloads","issues_url":"https://api.github.com/repos/dnephin/pyramid_swagger/issues{/number}","pulls_url":"https://api.github.com/repos/dnephin/pyramid_swagger/pulls{/number}","milestones_url":"https://api.github.com/repos/dnephin/pyramid_swagger/milestones{/number}","notifications_url":"https://api.github.com/repos/dnephin/pyramid_swagger/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/dnephin/pyramid_swagger/labels{/name}","releases_url":"https://api.github.com/repos/dnephin/pyramid_swagger/releases{/id}","created_at":"2015-02-18T22:49:53Z","updated_at":"2015-02-18T22:49:54Z","pushed_at":"2015-02-21T23:14:32Z","git_url":"git://github.com/dnephin/pyramid_swagger.git","ssh_url":"git@github.com:dnephin/pyramid_swagger.git","clone_url":"https://github.com/dnephin/pyramid_swagger.git","svn_url":"https://github.com/dnephin/pyramid_swagger","homepage":"http://pyramid-swagger.readthedocs.org/","size":257,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"striglia:master","ref":"master","sha":"278b2c1d9a72d54748045e8de42a6d53e4c3e24e","user":{"login":"striglia","id":262630,"avatar_url":"https://avatars.githubusercontent.com/u/262630?v=3","gravatar_id":"","url":"https://api.github.com/users/striglia","html_url":"https://github.com/striglia","followers_url":"https://api.github.com/users/striglia/followers","following_url":"https://api.github.com/users/striglia/following{/other_user}","gists_url":"https://api.github.com/users/striglia/gists{/gist_id}","starred_url":"https://api.github.com/users/striglia/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/striglia/subscriptions","organizations_url":"https://api.github.com/users/striglia/orgs","repos_url":"https://api.github.com/users/striglia/repos","events_url":"https://api.github.com/users/striglia/events{/privacy}","received_events_url":"https://api.github.com/users/striglia/received_events","type":"User","site_admin":false},"repo":{"id":20118512,"name":"pyramid_swagger","full_name":"striglia/pyramid_swagger","owner":{"login":"striglia","id":262630,"avatar_url":"https://avatars.githubusercontent.com/u/262630?v=3","gravatar_id":"","url":"https://api.github.com/users/striglia","html_url":"https://github.com/striglia","followers_url":"https://api.github.com/users/striglia/followers","following_url":"https://api.github.com/users/striglia/following{/other_user}","gists_url":"https://api.github.com/users/striglia/gists{/gist_id}","starred_url":"https://api.github.com/users/striglia/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/striglia/subscriptions","organizations_url":"https://api.github.com/users/striglia/orgs","repos_url":"https://api.github.com/users/striglia/repos","events_url":"https://api.github.com/users/striglia/events{/privacy}","received_events_url":"https://api.github.com/users/striglia/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/striglia/pyramid_swagger","description":"Convenient tools for using Swagger to define and validate your interfaces in a Pyramid webapp.","fork":false,"url":"https://api.github.com/repos/striglia/pyramid_swagger","forks_url":"https://api.github.com/repos/striglia/pyramid_swagger/forks","keys_url":"https://api.github.com/repos/striglia/pyramid_swagger/keys{/key_id}","collaborators_url":"https://api.github.com/repos/striglia/pyramid_swagger/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/striglia/pyramid_swagger/teams","hooks_url":"https://api.github.com/repos/striglia/pyramid_swagger/hooks","issue_events_url":"https://api.github.com/repos/striglia/pyramid_swagger/issues/events{/number}","events_url":"https://api.github.com/repos/striglia/pyramid_swagger/events","assignees_url":"https://api.github.com/repos/striglia/pyramid_swagger/assignees{/user}","branches_url":"https://api.github.com/repos/striglia/pyramid_swagger/branches{/branch}","tags_url":"https://api.github.com/repos/striglia/pyramid_swagger/tags","blobs_url":"https://api.github.com/repos/striglia/pyramid_swagger/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/striglia/pyramid_swagger/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/striglia/pyramid_swagger/git/refs{/sha}","trees_url":"https://api.github.com/repos/striglia/pyramid_swagger/git/trees{/sha}","statuses_url":"https://api.github.com/repos/striglia/pyramid_swagger/statuses/{sha}","languages_url":"https://api.github.com/repos/striglia/pyramid_swagger/languages","stargazers_url":"https://api.github.com/repos/striglia/pyramid_swagger/stargazers","contributors_url":"https://api.github.com/repos/striglia/pyramid_swagger/contributors","subscribers_url":"https://api.github.com/repos/striglia/pyramid_swagger/subscribers","subscription_url":"https://api.github.com/repos/striglia/pyramid_swagger/subscription","commits_url":"https://api.github.com/repos/striglia/pyramid_swagger/commits{/sha}","git_commits_url":"https://api.github.com/repos/striglia/pyramid_swagger/git/commits{/sha}","comments_url":"https://api.github.com/repos/striglia/pyramid_swagger/comments{/number}","issue_comment_url":"https://api.github.com/repos/striglia/pyramid_swagger/issues/comments{/number}","contents_url":"https://api.github.com/repos/striglia/pyramid_swagger/contents/{+path}","compare_url":"https://api.github.com/repos/striglia/pyramid_swagger/compare/{base}...{head}","merges_url":"https://api.github.com/repos/striglia/pyramid_swagger/merges","archive_url":"https://api.github.com/repos/striglia/pyramid_swagger/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/striglia/pyramid_swagger/downloads","issues_url":"https://api.github.com/repos/striglia/pyramid_swagger/issues{/number}","pulls_url":"https://api.github.com/repos/striglia/pyramid_swagger/pulls{/number}","milestones_url":"https://api.github.com/repos/striglia/pyramid_swagger/milestones{/number}","notifications_url":"https://api.github.com/repos/striglia/pyramid_swagger/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/striglia/pyramid_swagger/labels{/name}","releases_url":"https://api.github.com/repos/striglia/pyramid_swagger/releases{/id}","created_at":"2014-05-24T00:43:16Z","updated_at":"2015-02-19T09:46:31Z","pushed_at":"2015-01-29T01:02:47Z","git_url":"git://github.com/striglia/pyramid_swagger.git","ssh_url":"git@github.com:striglia/pyramid_swagger.git","clone_url":"https://github.com/striglia/pyramid_swagger.git","svn_url":"https://github.com/striglia/pyramid_swagger","homepage":"http://pyramid-swagger.readthedocs.org/","size":758,"stargazers_count":13,"watchers_count":13,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":14,"mirror_url":null,"open_issues_count":21,"forks":14,"open_issues":21,"watchers":13,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/82"},"html":{"href":"https://github.com/striglia/pyramid_swagger/pull/82"},"issue":{"href":"https://api.github.com/repos/striglia/pyramid_swagger/issues/82"},"comments":{"href":"https://api.github.com/repos/striglia/pyramid_swagger/issues/82/comments"},"review_comments":{"href":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/82/comments"},"review_comment":{"href":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/striglia/pyramid_swagger/pulls/82/commits"},"statuses":{"href":"https://api.github.com/repos/striglia/pyramid_swagger/statuses/473b5d5d1219f15f1d4aa7c2fd5007ccab48d92e"}}}}