Delegate spec validation to `swagger_spec_validator`. #86

prat0318 · 2015-03-03T21:35:53Z

Fixes #81. This commit delegates swagger validation to py package
swagger_spec_validator. This commit only covers the validation part of
swagger 1.2/2.0 spec and in no way handles the serving of 2.0 compatible endpoint.

It currently breaks py33 and py34 tests. This should be rectified with
the merge of Yelp/swagger_spec_validator#11.

Another change made is regarding the swagger spec file convention between 1.2 and 2.0.
Validation check first tries to find Swagger 2.0 spec (which should be just a single file, named, swagger.json) otherwise it tries to find Swagger 1.2 spec. swagger_spec_validator reads the spec again to decide whether the spec is a 1.2 or a 2.0. I am checking these here because, convention wise, api_docs.json is not a correct name for a 2.0 swagger schema. More such changes would need to be made when pyramid_swagger handles 2.0 schema completely.

coveralls · 2015-03-03T21:37:51Z

Changes Unknown when pulling 8b90e1a on prat0318:introduce_ssv into * on striglia:master*.

dnephin · 2015-03-03T21:46:08Z

tests/api_test.py

@@ -24,25 +22,24 @@ def test_basepath_rewriting():
 def build_config(schema_dir):
    return mock.Mock(
        registry=get_registry({
-            'swagger_schema': compile_swagger_schema(schema_dir),
+            'swagger_schema': validate_swagger_schema(schema_dir),
        })


I don't think this is right validate_swagger_schema doesn't return anything. This should still be compile_swagger_schema.

pyramid_swagger should still return the same exceptions, we probably just want to wrap the exception

Oh correct. I was testing my validation spec here which was wrong. I have reverted back these changes, and added similar tests in spec_test.py which get validated through swagger_spec_validator.

dnephin · 2015-03-03T21:47:27Z

This mostly lgtm, but I think the test changes show that we actually need to wrap some exceptions. Users should expect the same exceptions out of pyramid_swagger.

swagger_spec_validator. This commit only covers validation of swagger 2.0 spec and in no way handles serving of 2.0 compatible endpoint. It currently breaks py33 and py34 tests. This should be rectified with the merge of Yelp/swagger_spec_validator#11.

coveralls · 2015-03-03T22:06:05Z

Changes Unknown when pulling 94c50a3 on prat0318:introduce_ssv into * on striglia:master*.

prat0318 · 2015-03-03T22:09:53Z

Currently the possible exposed exceptions are ResourceListingNotFoundError, ApiDeclarationNotFoundError and SwaggerValidationError. Either we can use the third one to wrap the first two, or wrap ssv's SwaggerValidationError to some pyramid_swagger's SwaggerValidationError. I will create a separate issue to track it.

striglia · 2015-03-03T22:13:57Z

Yeah I'd focus on backwards compatibility right now, but keep in mind we're free to have more descriptive exceptions once we do a breaking change. My guess is users won't find distinct exception classes for the various "your spec is invalid" exceptions all that useful....we can probably just use one consistent exception and different messages.

prat0318 · 2015-03-03T22:19:41Z

This patch is BC, except that jsonschema.ValidationError error thrown is now been replaced with swagger_spec_validator.SwaggerValidationError. Do we want to wrap the later error with the former for this patch to maintain BC?

landscape-bot · 2015-03-03T22:22:09Z

Code quality remained the same when pulling 94c50a3 on prat0318:introduce_ssv into 425fcfa on striglia:master.

landscape-bot · 2015-03-03T22:24:28Z

Code quality remained the same when pulling 94c50a3 on prat0318:introduce_ssv into 425fcfa on striglia:master.

striglia · 2015-03-04T19:35:24Z

Yeah let's wrap it up to maintain backwards compatibility. I don't mind if exception msg changes, but the class should be consistent. You are free to modify the message to make it clear what is going wrong.

prat0318 · 2015-03-04T20:08:49Z

Made the change BC by wrapping the ssv's error with jsonschema's ValidationError. A subtle difference still remains is that if a resource_listing file is not present in the schema_dir, earlier IOError was thrown, but now jsonschema's ValidationError will be thrown (as all ssv's exceptions are wrapped under ValidationError), but i don't think that will be a showstopper.

landscape-bot · 2015-03-04T20:10:33Z

Repository health decreased by 0.07% when pulling b68dd8e on prat0318:introduce_ssv into 425fcfa on striglia:master.

1 new problem was found (including 0 errors and 0 code smells).
No problems were fixed.

striglia · 2015-03-04T22:46:24Z

That subtle difference is probably fine. I want to revamp these errors in 2.0.0 but for now we'll let it slide. Looks good aside from the CI build passing.

striglia · 2015-03-04T22:47:58Z

pyramid_swagger/exceptions.py

@@ -8,3 +11,12 @@ class RequestValidationError(HTTPClientError):

 class ResponseValidationError(HTTPInternalServerError):
    pass
+
+
+def wrap_exception(method):


One thought: let's name this specifically or pass in the exception to reraise as.

Good idea. i will pass the exception to reraise as an arg.

analogue · 2015-03-06T19:55:32Z

+1 "we can probably just use one consistent exception and different messages."

analogue · 2015-03-06T20:00:39Z

pyramid_swagger/spec.py

+            'No swagger spec found in directory:{0}. Note that your json file '
+            'must be named {1} or {2}'.format(
+                schema_dir, SWAGGER_2DOT0_FILENAME, API_DOCS_FILENAME))
+


Possible to remove the else stmt.

if x: return swagger20_filepath if y: return swagger12_filepath raise swagger_spec_validator.SwaggerValidationError(...)

…r to maintain backwards compatiblity.

striglia · 2015-03-25T22:30:09Z

@prat0318 is this ready or still in progress?

dnephin · 2015-03-26T18:28:57Z

This is kind of blocked on the jsonschema changes as well, to fix the py3 errors.

prat0318 · 2015-03-26T19:11:40Z

Yeah, blocked specifically on python-jsonschema/jsonschema#203 getting merged.

landscape-bot · 2015-04-15T19:56:14Z

Repository health increased by 0.07% when pulling 07bb136 on prat0318:introduce_ssv into e57b344 on striglia:master.

2 new problems were found (including 0 errors and 1 code smell).
2 problems were fixed (including 0 errors and 1 code smell).

…roduce_ssv Conflicts: pyramid_swagger/__init__.py pyramid_swagger/ingest.py pyramid_swagger/spec.py tests/api_test.py tests/includeme_test.py

landscape-bot · 2015-04-15T22:56:58Z

Repository health decreased by 0.23% when pulling 068f4cb on prat0318:introduce_ssv into fe3b877 on striglia:master.

2 new problems were found (including 0 errors and 1 code smell).
No problems were fixed.

prat0318 · 2015-04-15T23:02:48Z

tests/sample_schemas/bad_app/bad_sample.json

@@ -14,7 +14,8 @@
                        {
                            "paramType": "query",
                            "name": "content",
-                            "required": true
+                            "required": true,
+                            "type": "string"


type: string was added as jsonschema was flaking with no type present error instead of intended nickname not present error.

…gger_spec_validator

landscape-bot · 2015-04-15T23:06:16Z

Repository health decreased by 0.23% when pulling bda97cd on prat0318:introduce_ssv into fe3b877 on striglia:master.

2 new problems were found (including 0 errors and 1 code smell).
No problems were fixed.

coveralls · 2015-04-15T23:07:43Z

Coverage increased (+0.0%) to 99.72% when pulling bda97cd on prat0318:introduce_ssv into fe3b877 on striglia:master.

coveralls · 2015-04-15T23:07:43Z

Coverage increased (+0.0%) to 99.72% when pulling bda97cd on prat0318:introduce_ssv into fe3b877 on striglia:master.

prat0318 · 2015-04-15T23:12:24Z

pyramid_swagger/load_schema.py

@@ -16,6 +16,7 @@


 EXTENDED_TYPES = {
+    'number': (float,),


'float': (float) expected the param type to be 'float', which was not as per swagger spec 1.2 (the format can be float, not the type).

prat0318 · 2015-04-15T23:15:44Z

@dnephin @analogue there were few merge conflicts with master, which i resolved. There is a float correction and sample tests spec correction as well in this commit. lmk if it looks good to merge.

analogue · 2015-04-17T16:02:04Z

pyramid_swagger/exceptions.py

+import sys
+
+import six
+


no blank line since six and pyramid are both 3rd party

analogue · 2015-04-17T16:53:24Z

Fix-n-ship

coveralls · 2015-04-20T19:49:20Z

Coverage increased (+0.0%) to 99.72% when pulling 092e095 on prat0318:introduce_ssv into fe3b877 on striglia:master.

coveralls · 2015-04-20T19:49:21Z

Coverage increased (+0.0%) to 99.72% when pulling 092e095 on prat0318:introduce_ssv into fe3b877 on striglia:master.

coveralls · 2015-04-20T19:49:23Z

Coverage increased (+0.0%) to 99.72% when pulling 092e095 on prat0318:introduce_ssv into fe3b877 on striglia:master.

landscape-bot · 2015-04-20T19:51:01Z

Repository health decreased by 0.22% when pulling 092e095 on prat0318:introduce_ssv into fe3b877 on striglia:master.

2 new problems were found (including 0 errors and 1 code smell).
No problems were fixed.

coveralls · 2015-04-23T22:53:05Z

Coverage decreased (-0.0%) to 99.72% when pulling 9f0cfe7 on prat0318:introduce_ssv into fe3b877 on striglia:master.

coveralls · 2015-04-23T22:53:06Z

Coverage decreased (-0.0%) to 99.72% when pulling 9f0cfe7 on prat0318:introduce_ssv into fe3b877 on striglia:master.

landscape-bot · 2015-04-23T22:56:51Z

Repository health decreased by 0.29% when pulling 9f0cfe7 on prat0318:introduce_ssv into fe3b877 on striglia:master.

2 new problems were found (including 0 errors and 1 code smell).
No problems were fixed.

dnephin · 2015-04-27T19:13:40Z

Looks good!

Delegate spec validation to `swagger_spec_validator`.

dnephin reviewed Mar 3, 2015
View reviewed changes

prat0318 force-pushed the introduce_ssv branch from 8b90e1a to 94c50a3 Compare March 3, 2015 22:03

striglia reviewed Mar 4, 2015
View reviewed changes

analogue reviewed Mar 6, 2015
View reviewed changes

Wrap SwaggerValidationError with jsonschema.Exceptions.ValidationErro…

e556363

…r to maintain backwards compatiblity.

prat0318 force-pushed the introduce_ssv branch from b68dd8e to e556363 Compare March 9, 2015 20:26

1) Pin ssv to >= 1.0.10 and update syntax to be py3 compatible

07bb136

Merge branch 'master' of github.com:striglia/pyramid_swagger into int…

068f4cb

…roduce_ssv Conflicts: pyramid_swagger/__init__.py pyramid_swagger/ingest.py pyramid_swagger/spec.py tests/api_test.py tests/includeme_test.py

prat0318 reviewed Apr 15, 2015
View reviewed changes

Correct swagger 1.2 spec as it started failing the validations of swa…

bda97cd

…gger_spec_validator

prat0318 reviewed Apr 15, 2015
View reviewed changes

analogue reviewed Apr 17, 2015
View reviewed changes

Add spec param when assigning Mock objects

092e095

Change validation to now entertain only v1.2 spec

9f0cfe7

dnephin added a commit that referenced this pull request Apr 27, 2015

Merge pull request #86 from prat0318/introduce_ssv

d218f96

Delegate spec validation to `swagger_spec_validator`.

dnephin merged commit d218f96 into Yelp:master Apr 27, 2015

		import sys

		import six

Delegate spec validation to swagger_spec_validator. #86

Delegate spec validation to swagger_spec_validator. #86

Uh oh!

Conversation

prat0318 commented Mar 3, 2015

Uh oh!

coveralls commented Mar 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnephin commented Mar 3, 2015

Uh oh!

coveralls commented Mar 3, 2015

Uh oh!

prat0318 commented Mar 3, 2015

Uh oh!

striglia commented Mar 3, 2015

Uh oh!

prat0318 commented Mar 3, 2015

Uh oh!

landscape-bot commented Mar 3, 2015

Uh oh!

landscape-bot commented Mar 3, 2015

Uh oh!

striglia commented Mar 4, 2015

Uh oh!

prat0318 commented Mar 4, 2015

Uh oh!

landscape-bot commented Mar 4, 2015

Uh oh!

striglia commented Mar 4, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

analogue commented Mar 6, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

striglia commented Mar 25, 2015

Uh oh!

dnephin commented Mar 26, 2015

Uh oh!

prat0318 commented Mar 26, 2015

Uh oh!

landscape-bot commented Apr 15, 2015

Uh oh!

landscape-bot commented Apr 15, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

landscape-bot commented Apr 15, 2015

Uh oh!

coveralls commented Apr 15, 2015

Uh oh!

coveralls commented Apr 15, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prat0318 commented Apr 15, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

analogue commented Apr 17, 2015

Uh oh!

coveralls commented Apr 20, 2015

Uh oh!

coveralls commented Apr 20, 2015

Uh oh!

coveralls commented Apr 20, 2015

Uh oh!

landscape-bot commented Apr 20, 2015

Uh oh!

Delegate spec validation to `swagger_spec_validator`. #86

Delegate spec validation to `swagger_spec_validator`. #86