Make api docs route optional

[dnephin]

I believe this resolves #73

2b092b1 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.

0a44a58 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.

c259561 addresses the first point of #73. Adds an option to completely disable /api-docs registration, leaving it up to the user. Docs updated.

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).

This 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.

This 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.

[dnephin]

and I just realized I was working off an old master... so I'm going to have some work to do rebasing this

[dnephin]

rebase wasn't as bad as I thought

[coveralls]

Coverage remained the same at 100.0% when pulling e5d40cc on dnephin:make_api_docs_route_optional into 278b2c1 on striglia:master.

[coveralls]

Coverage remained the same at 100.0% when pulling a72a1f4 on dnephin:make_api_docs_route_optional into 278b2c1 on striglia:master.

[dnephin]

Hmm, I can't reproduce either of those test failures locally

[coveralls]

Coverage remained the same at 100.0% when pulling bb38a7f on dnephin:make_api_docs_route_optional into 278b2c1 on striglia:master.

[coveralls]

Coverage remained the same at 100.0% when pulling f3b94cd on dnephin:make_api_docs_route_optional into 278b2c1 on striglia:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 2172daf on dnephin:make_api_docs_route_optional into 278b2c1 on striglia:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 2172daf on dnephin:make_api_docs_route_optional into 278b2c1 on striglia:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 473b5d5 on dnephin:make_api_docs_route_optional into 278b2c1 on striglia:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 5403fa8 on dnephin:make_api_docs_route_optional into 278b2c1 on striglia:master.

[dnephin]

Ok, well I tried to run these tests with the same version of tox, using the same hashseed and it still doesn't fail for me locally. Unless someone has some idea of what's going on, I'm going to start looking at changing these tests. My branch should not have modified the behaviour of this function at all (and they pass for other python versions).

[striglia]

Whoa. Weird. Feel free to change those tests as needed...the intent should be pretty clear.

[prat0318]

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.

[dnephin]

The reason I don't like that as much is because it makes the validation kind of implicit instead of being it's own call it's just a side-effect of building a schema.

Does ssv need build_schema_mapping() ? I was thinking that it would work off just listing_filename, which is why I left the duplicate lines.

[prat0318]

Oh yeah, i missed that. ssv would just need the listing_filename for its purpose.

[striglia]

+1 to not exposing the schema mapping data structure to the outside world. I've explicitly avoided making it public so we have refactoring flexibility going forward. In the event pyramid_swagger is ever 2.0-only, I bet we could (and would want to) do a major refactoring of the data structure itself.

[dnephin]

I was finally able to reproduce this failure on a different laptop

(Pdb) print paths
[<_sre.SRE_Pattern object at 0x7ff4a70cb6b0>, <_sre.SRE_Pattern object at 0x7ff4a6fa71f0>]
(Pdb) print expected
[<_sre.SRE_Pattern object at 0x7ff4a70cb780>, <_sre.SRE_Pattern object at 0x7ff4a6fa71f0>]
(Pdb) paths[0] == expected[0]
False
(Pdb) paths[1] == expected[1]
True
(Pdb) paths[0].pattern
u'^/static/?'
(Pdb) expected[0].pattern
u'^/static/?'
(Pdb) paths[0].pattern == expected[0].pattern
True

I believe these tests are relying on an undefined behavior of re to cache the compiled regex. If it hits the cache the test passed, but if it doesn't hit the cache, the test fails. I'm thinking of changing the test to compare against the .pattern instead of the object.

[coveralls]

Coverage remained the same at 100.0% when pulling b9ff48a on dnephin:make_api_docs_route_optional into 278b2c1 on striglia:master.

[striglia]

Let's put a comment explaining the purpose of stuffing this in here.

[striglia]

Tweak and I'll merge once tests pass

[dnephin]

merged and tests passing!