Add "ParsedMethodView" and example #222

strongbugman · 2018-08-01T08:36:25Z

@rochacbruno I don't kwon if the file/class name/structure is suitable, feel free to tell me if I'm wrong.

coveralls · 2018-08-01T08:39:05Z

Coverage increased (+0.9%) to 91.184% when pulling be670d6 on strongbugman:feature/add_parsed_view into 559764e on rochacbruno:master.

rochacbruno · 2018-08-10T17:35:25Z

This is a great improvement, I'll try to test and review by this weekend!

If you can also take a look @javabrett :)

rochacbruno · 2018-08-10T17:40:56Z

I Like the implementation with the ParseMethodView as a base class.

But I also think we can provide the same functionality for when only function based views are used, and also when no viewa at all are used. (this would address the issues #202 and #216

Example:

Swagger(.....parse_spec_path="path/to/docs")

Then we just parse the YAML and generate the swagger UI even without having any view defined.

strongbugman · 2018-08-11T06:53:47Z

Thanks your suggestion, you are right, I will update this PR soon

strongbugman · 2018-08-15T15:23:03Z

@rochacbruno You can review this PR now, the base idea is same with earlier "ParseMethodView", but it has more applicability:

Auto find view function/method's swagger docs file by endpoint's name when "doc_dir" is configured
Parse and validate all documented endpoint's request when pass "parse=True" to flasgger app.
Move most code from APISpecsView to Flasgger, now we can get spec json by Flasgger.get_apispecs

Any suggestions or criticisms are welcome

rochacbruno · 2018-08-15T19:47:09Z

@strongbugman this is awesome! I'll review ASAP!

javabrett

I know this is fairly current, but please rebase to current master for final review.

From http://localhost:5000/parsed_view_func/apidocs/#/item/get_api_items__id__ I' I'm getting this error:

Errors
Hide
Resolver error at paths./api/items/{id}/.get.responses.200.schema.$ref
Could not resolve reference because of: Could not resolve pointer: /definitions/item does not exist in document

javabrett · 2018-08-17T00:35:38Z

examples/parsed_view_func.py

@@ -0,0 +1,170 @@
+"""
+Example of auto load doc file by endpoint name(view function/method name),
+parse by flask_restful.reqparse.RequestParser and validate by jsonschema


s/parse/parsed
s/validate/validated

javabrett · 2018-08-17T00:36:28Z

examples/parsed_view_func.py

+from flasgger import Swagger
+
+
+_TEST_META_SKIP_FULL_VALIDATION = True


Is this active/needed, what does it do?

No, it should be removed

javabrett · 2018-08-17T00:37:39Z

examples/parsed_view_func.py

+    def get(self):
+        """
+        If we set "parse" is True in Flasgger app, we will get parsed and
+        validate data store in "flask.request".


s/validate/validated
s/store/stored

javabrett · 2018-08-17T00:38:03Z

examples/parsed_view_func.py

+        If we set "parse" is True in Flasgger app, we will get parsed and
+        validate data store in "flask.request".
+
+        Different location`s var store in different key, eg: key "args" store


Maybe ` -> ' here?

strongbugman · 2018-08-17T03:52:47Z

@javabrett Thx for your review, I fixed the errors and rebased the origin/master

javabrett · 2018-08-17T05:08:38Z

Changes look good, 🍰 . I don't think your rebase picked up the latest master but never mind - you might have forgotten to fetch or use origin/master.

strongbugman · 2018-08-17T05:19:12Z

@javabrett Yes you are right, I forgot to fetch upstream😅, now it's rebased

rochacbruno · 2018-08-21T19:58:32Z

@strongbugman awesome!

@javabrett first, thanks for the awesome contribution to Flasgger reviewing and helping maintaining this project ✨

@javabrett I am with little time this week to review OSS projects (due to work + FlaskConf which is this weekend). So I would like to ask if you can take care of this PR! this is awesome improvement and I do not want to hold it. As long as the code is good, working and we have some kind of doc (readme) about how to use it please decide if you want to merge it or request more changes! I'll let this one for you to review/merge!

:) thanks

javabrett

@strongbugman I don't have large blocks of time to work on the review at the moment, so hopefully you can live with my incremental suggestions :).

Left a comment on the auth in the example below.

javabrett · 2018-08-21T23:13:05Z

examples/parsed_view_func.py

+      400:
+        description: Miss name or code
+    """
+    if request.parsed_data['json']['data']['password'] != 'test':


Regarding this, and related auth bits - I'm just wondering whether this is an essential part of the example code, or whether is it orthogonal and potentially confusing.

With the password, I had to check the example source to find it. It also seems a bit irregular auth passing the credentials with the data payload.

If auth is integral to the example, fine let's keep it, but if it is unnecessary and orthogonal, maybe we can have auth in other examples but not here. Thoughts?

When you next rebase you'll pick up the new Dockerfile which can be useful for testing.

This example is a test about nest json body and variable route rule without converter, but the "auth" feature is unnecessary, so I`ll change the example

javabrett · 2018-08-21T23:34:09Z

examples/parsed_view_func.py

+
+app = Flask(__name__)
+app.config['SWAGGER'] = {
+    'title': 'Flasgger ParsedMethodView Example',


Regarding ParsedMethodView (three mentions, appears in UI) - on first seeing this in camel-case, I wondered if it was going to be a subclass of Flask MethodView, but it's not, it's just a name used in the example. So maybe "Flasgger Parsed MethodView Example" might be better?

javabrett · 2018-08-22T01:11:21Z

flasgger/base.py

+        # "static_folder": "static",  # must be set by user
+        "swagger_ui": True,
+        "specs_route": "/apidocs/",
+        "doc_dir": "./docs/api/"


Doc-parsing is new and presumably optional and opt-in, but doc_dir is being set in the DEFAULT_CONFIG here. As a result all existing examples are passing through the new code in get_specs, and the doc_dir=None is not having any effect.

So I guess I'm checking whether it is really intended to set this as a default here. If not, and especially if all other options are mentioned here, it might be OK to leave it commented per static_folder.

If the intention is to make this new feature default-on with some default path to the docs, then the parse option would need review.

Doc auto loading is a optional feature, although it won't affect flasgger's other feature if there is no ./docs/api in local directory, for more clearly, I'll remove the doc_dir config in DEFAULT_CONFIG

strongbugman · 2018-08-22T08:29:34Z

@javabrett Thx for your suggestions and patience, I changed the code according to your suggestions

request with documented

javabrett · 2018-08-27T02:40:52Z

Thanks @strongbugman for this contribution! 🍰

rochacbruno requested a review from javabrett August 10, 2018 17:34

strongbugman force-pushed the feature/add_parsed_view branch 12 times, most recently from 222caa6 to eb3c622 Compare August 15, 2018 15:18

javabrett requested changes Aug 17, 2018

View reviewed changes

javabrett requested a review from rochacbruno August 17, 2018 00:46

strongbugman force-pushed the feature/add_parsed_view branch from eb3c622 to 6509cfb Compare August 17, 2018 03:31

javabrett approved these changes Aug 17, 2018

View reviewed changes

strongbugman force-pushed the feature/add_parsed_view branch from 6509cfb to 76466c0 Compare August 17, 2018 05:14

javabrett self-assigned this Aug 21, 2018

javabrett requested changes Aug 21, 2018

View reviewed changes

javabrett requested changes Aug 22, 2018

View reviewed changes

strongbugman force-pushed the feature/add_parsed_view branch from 76466c0 to 45f1893 Compare August 22, 2018 08:21

Auto load swag yml file for view function and method, auto parse all

be670d6

request with documented

strongbugman force-pushed the feature/add_parsed_view branch from 45f1893 to be670d6 Compare August 22, 2018 09:32

javabrett approved these changes Aug 22, 2018

View reviewed changes

javabrett merged commit aaef05c into flasgger:master Aug 27, 2018

This was referenced Aug 27, 2018

How to reduce code when register paths to template #202

Closed

Exporting swagger spec JSON file programmatically #216

Closed

strongbugman mentioned this pull request Aug 27, 2018

Validate query parameters #217

Open

wangyangjun mentioned this pull request Sep 19, 2019

Query parameters validation with data type as array #337

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "ParsedMethodView" and example #222

Add "ParsedMethodView" and example #222

strongbugman commented Aug 1, 2018

coveralls commented Aug 1, 2018 •

edited

Loading

rochacbruno commented Aug 10, 2018

rochacbruno commented Aug 10, 2018 •

edited

Loading

strongbugman commented Aug 11, 2018

strongbugman commented Aug 15, 2018

rochacbruno commented Aug 15, 2018

javabrett left a comment

javabrett Aug 17, 2018

strongbugman Aug 17, 2018

javabrett Aug 17, 2018

strongbugman Aug 17, 2018

javabrett Aug 17, 2018

strongbugman Aug 17, 2018

javabrett Aug 17, 2018

strongbugman commented Aug 17, 2018

javabrett commented Aug 17, 2018

strongbugman commented Aug 17, 2018

rochacbruno commented Aug 21, 2018

javabrett left a comment

javabrett Aug 21, 2018 •

edited

Loading

strongbugman Aug 22, 2018

javabrett Aug 21, 2018

javabrett Aug 22, 2018

strongbugman Aug 22, 2018

strongbugman commented Aug 22, 2018

javabrett commented Aug 27, 2018

		from flasgger import Swagger


		_TEST_META_SKIP_FULL_VALIDATION = True

Add "ParsedMethodView" and example #222

Add "ParsedMethodView" and example #222

Conversation

strongbugman commented Aug 1, 2018

coveralls commented Aug 1, 2018 • edited Loading

rochacbruno commented Aug 10, 2018

rochacbruno commented Aug 10, 2018 • edited Loading

strongbugman commented Aug 11, 2018

strongbugman commented Aug 15, 2018

rochacbruno commented Aug 15, 2018

javabrett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strongbugman commented Aug 17, 2018

javabrett commented Aug 17, 2018

strongbugman commented Aug 17, 2018

rochacbruno commented Aug 21, 2018

javabrett left a comment

Choose a reason for hiding this comment

javabrett Aug 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strongbugman commented Aug 22, 2018

javabrett commented Aug 27, 2018

coveralls commented Aug 1, 2018 •

edited

Loading

rochacbruno commented Aug 10, 2018 •

edited

Loading

javabrett Aug 21, 2018 •

edited

Loading