initial implementation of OAS 3.0 generateschema #689
Conversation
|
I found a mistake in the tox.ini/.travis changes -- commit coming soon as I finish testing locally. |
Codecov Report
@@ Coverage Diff @@
## master #689 +/- ##
==========================================
- Coverage 95.96% 93.26% -2.71%
==========================================
Files 54 58 +4
Lines 2728 3161 +433
==========================================
+ Hits 2618 2948 +330
- Misses 110 213 +103
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #689 +/- ##
==========================================
- Coverage 95.97% 95.92% -0.06%
==========================================
Files 54 59 +5
Lines 2735 3138 +403
==========================================
+ Hits 2625 3010 +385
- Misses 110 128 +18
Continue to review full report at Codecov.
|
|
@arielpontes I'd appreciate it if you could test this. Just add something like You can test the schema with something like Ignore the "DELETE operations cannot have a requestBody." messages. |
There was a problem hiding this comment.
Thanks for working on this.
This is quite a massive PR and very hard to review, so we need to make it smaller to get started. All in all when looking through it I have a feeling not all code is jsonapi specific.
I haven't dived into how generateschema works exactly so correct me if my suggestions do not seem to be valid.
Suggestions:
- use snapshottest to simplify large asserts in tests
- we could remove support for DRF 3.9 in master to make things simpler here. What do you think? (I don't see that now 3.10 is released and in DJA release 3.0.0 we remove almost all DRF releases why we should continue supporting 3.9)
- oauth and rest_condition code resp. security code in general isn't jsonapi specification right? If so I rather we delete it from this PR. We can then still discuss at a later point where the code should best go.
If you have other ideas how we could start integrating this change with small PRs please feel free to comment. Above suggestions should at least make it simpler.
|
@sliverc Thanks for the the feedback. I had intended to implement OAS here first before moving portions of the implementation that are commonly useful (like securitySchemes and security objects) to DRF PRs as Tom has indicated he wants to wait to see what implementations happen before making too many more changes there. I can look at doing that now and, in general, shrinking this into a more manageable PR. Thanks for the snapshottest pointer. Removing DRF 3.9 support sounds good. Should we make a 3.0 feature branch here or just drop 3.9 from the master branch? |
|
Let's simply drop DRF 3.9 support from master in another PR |
|
Getting this when trying to run 'generateschema': |
Please share your INSTALLED_APPS from settings.py. I think this is because 'rest_framework_json_api' is not before 'rest_framework' per the updated documentation |
|
Thanks for answering, but nope, my INSTALLED_APPS seem to be alright: If I put it in the wrong order, as you described, there's no error and it's just ignored. My REST_FRAMEWORK settings portion is straight from the linked docs, both serializers and views are imported from rest_framework_json_api. |
|
@andrew-snek Sorry, I missed documenting a step. Now add here Basically you need to make sure the DJA version of AutoSchema is used. |
|
No need to be sorry, you're doing this for free and I'm grateful. Now a question that technically goes beyond the scope of this pull request, but I think will be useful to many people: by chance, do you have any ideas how to make this work with drf-yasg - a package which autogenerates web ui for the schema? I see you've subclassed the schema generator in this PR, and now stuff like this fails with 403; permission_classes which allow everything seem to be ignored: |
|
@andrew-snek Sorry, I didn't even know This See an example of using this stuff (in an early pre-release form) here. This looks to be doing the same thing as drf-yasg. |
|
@sliverc I've rebased. Can you take another look please? |
- works around a bug in django.contrib.admindocs.utils.replace_named_groups that fails to replace a named group if there's no trailing / - only make the change to urls.py; urls_test.py has a bunch of tests that expect the / to be missing.
|
Hi, sorry I disappeared. I'm trying to run |
|
Check your Django-filters version. We need to add a minimum version for
this optional package I think.
Also, I’ve pushed several more commits with bugs I found while expanding
test coverage so please make sure to refresh the package.
…On Mon, Aug 26, 2019 at 5:55 AM Ariel Pontes ***@***.***> wrote:
Hi, sorry I disappeared. I'm trying to run generateschema now, I got some
errors and reading the thread I fixed some of them, also some were in my
code (getting stuff from self.request in some views was breaking, so I
added some getattrs and it did the job. I'm still getting this, however:
Traceback (most recent call last):
File "manage.py", line 30, in <module>
execute_from_command_line(sys.argv)
File "/usr/local/lib/python3.7/site-packages/django/core/management/__init__.py", line 381, in execute_from_command_line
utility.execute()
File "/usr/local/lib/python3.7/site-packages/django/core/management/__init__.py", line 375, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/usr/local/lib/python3.7/site-packages/django/core/management/base.py", line 323, in run_from_argv
self.execute(*args, **cmd_options)
File "/usr/local/lib/python3.7/site-packages/django/core/management/base.py", line 364, in execute
output = self.handle(*args, **options)
File "/usr/local/lib/python3.7/site-packages/rest_framework/management/commands/generateschema.py", line 40, in handle
schema = generator.get_schema(request=None, public=True)
File "/usr/local/lib/python3.7/site-packages/rest_framework/schemas/openapi.py", line 64, in get_schema
paths = self.get_paths(None if public else request)
File "/usr/local/lib/python3.7/site-packages/rest_framework/schemas/openapi.py", line 47, in get_paths
operation = view.schema.get_operation(path, method)
File "/usr/local/lib/python3.7/site-packages/rest_framework_json_api/schemas/openapi.py", line 466, in get_operation
parameters += self._get_filter_parameters(path, method)
File "/usr/local/lib/python3.7/site-packages/rest_framework/schemas/openapi.py", line 187, in _get_filter_parameters
parameters += filter_backend().get_schema_operation_parameters(self.view)
File "/usr/local/lib/python3.7/site-packages/rest_framework_json_api/django_filters/backends.py", line 135, in get_schema_operation_parameters
for res in super(DjangoFilterBackend, self).get_schema_operation_parameters(view):
AttributeError: 'super' object has no attribute 'get_schema_operation_parameters'
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#689?email_source=notifications&email_token=ABBHS57DA2QOIRE3CBXQU6DQGOR7VA5CNFSM4ILE7AL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5D425Y#issuecomment-524799351>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABBHS56KSPVRQDOROWJTMGTQGOR7VANCNFSM4ILE7ALQ>
.
|
|
@n2ygk I've updated I'm still getting some errors in the Swagger UI though. This one appears multiple times: In the terminal where I ran the |
|
Can you provide a small test case that reproduces the error?
…On Tue, Aug 27, 2019 at 7:30 AM Ariel Pontes ***@***.***> wrote:
@n2ygk <https://github.com/n2ygk> I've updated django-filters from 2.1.0
to 2.2.0 and made a few other changes and the generateschema command is
now working :)
I'm still getting some errors in the Swagger UI though. This one appears
multiple times:
😱 Could not render ParameterRow, see the console.
In the terminal where I ran the swagger-ui-watcher command, I don't see
anything. In the UI page I see an error section at the bottom of the page
with a lot of error messages, too many to post here, but the first few look
like this:
Errors
Hide
Schema error should NOT have additional properties
additionalProperty: Loading
Jump to line 0
Schema error at paths['/api/v1/user-profiles/'].put
should have required property 'responses'
missingProperty: responses
Jump to line 0
Schema error at paths['/api/v1/organizations/{organization_id}/members/{id}/'].put
should have required property 'responses'
missingProperty: responses
Jump to line 0
Schema error at paths['/api/v1/organizations/invites/{id}/'].put
should have required property 'responses'
missingProperty: responses
Jump to line 0
Schema error at paths['/api/v1/organizations/{id}/'].put
should have required property 'responses'
missingProperty: responses
Jump to line 0
Schema error at paths['/api/v1/documents/{id}/'].put
should have required property 'responses'
missingProperty: responses
Jump to line 0
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#689?email_source=notifications&email_token=ABBHS57QRFJJZ7MDM3YQZ73QGUF5RA5CNFSM4ILE7AL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5HNLGA#issuecomment-525260184>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABBHS52H65WS6LZIQ4HMLWLQGUF5RANCNFSM4ILE7ALQ>
.
|
| @@ -15,3 +15,6 @@ recommonmark==0.6.0 | |||
| Sphinx==2.1.2 | |||
| sphinx_rtd_theme==0.4.3 | |||
| twine==1.13.0 | |||
| coreapi==2.3.3 | |||
There was a problem hiding this comment.
let's keep this file alphabetically ordered.
| for res in super(DjangoFilterBackend, self).get_schema_operation_parameters(view): | ||
| if 'name' in res: | ||
| res['name'] = 'filter[{}]'.format(res['name']).replace('__', '.') | ||
| result.append(res) |
There was a problem hiding this comment.
to create a new list but one line above to simply change a existing dictionary seems to be a bit odd.
I think it is properly easier especially as it is a super class call anyway to do something like this:
results = super(DjangoFilterBackend, self).get_schema_operation_parameters(view)
for result in results:
if 'name' in res:
result['name'] = 'filter[{}]'.format(result['name']).replace('__', '.')
return results| @@ -10,6 +10,8 @@ deps = | |||
| django22: Django>=2.2,<2.3 | |||
| drf310: djangorestframework>=3.10.2,<3.11 | |||
| drfmaster: https://github.com/encode/django-rest-framework/archive/master.zip | |||
| coreapi>=2.3.1 | |||
There was a problem hiding this comment.
no need to add this here. Below requirements-development.txt will be installed
There was a problem hiding this comment.
drf is going to drop coreapi
| 'type': 'object', | ||
| 'additionalProperties': True | ||
| }, | ||
| 'datum': { |
| @@ -15,3 +15,6 @@ recommonmark==0.6.0 | |||
| Sphinx==2.1.2 | |||
| sphinx_rtd_theme==0.4.3 | |||
| twine==1.13.0 | |||
| coreapi==2.3.3 | |||
| 'components': JSONAPI_COMPONENTS, | ||
| } | ||
|
|
||
| return {**schema, **self.openapi_schema} |
There was a problem hiding this comment.
is it not easier to call super and simply add components to the returned result?
| Extend DRF's SchemaGenerator to implement jsonapi-flavored generateschema command | ||
| """ | ||
| def __init__(self, *args, **kwargs): | ||
| self.openapi_schema = {} |
There was a problem hiding this comment.
DRF doesn't provide this - why do we introduce it in DJA?
|
hope to see this in soon |
|
Please do take over!
…On Thu, Feb 13, 2020 at 5:30 AM Kieran Evans ***@***.***> wrote:
@n2ygk <https://github.com/n2ygk> - Will you be able to address the
issues raised by @sliverc <https://github.com/sliverc> ? If not, we need
this, so happy to take over.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#689?email_source=notifications&email_token=ABBHS55K5XITFUFQV3WDTGTRCUONZA5CNFSM4ILE7AL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELUHBJQ#issuecomment-585658534>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABBHS5ZOKDJHT4O2K3E2ZMTRCUONZANCNFSM4ILE7ALQ>
.
|
Can do. Any notes to pass on? Will also need to bring things in line with the latest release of this lib. |
|
The openapi support in upstream DRF has matured a lot so I expect there may
need to be significant changes, hopefully reducing the amount of overridden
code.
I’ve just been too busy with other work to focus at all in this.
On Thu, Feb 13, 2020 at 8:07 AM Kieran Evans <notifications@github.com>
wrote:
… Please do take over!
… <#m_7495308537009845494_>
On Thu, Feb 13, 2020 at 5:30 AM Kieran Evans *@*.***> wrote: @n2ygk
<https://github.com/n2ygk> ***@***.*** <https://github.com/n2ygk>> - Will you
be able to address the issues raised by @sliverc
<https://github.com/sliverc> ***@***.*** <https://github.com/sliverc>> ? If
not, we need this, so happy to take over. — You are receiving this because
you were mentioned. Reply to this email directly, view it on GitHub <#689
<#689>?email_source=notifications&email_token=ABBHS55K5XITFUFQV3WDTGTRCUONZA5CNFSM4ILE7AL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELUHBJQ#issuecomment-585658534>,
or unsubscribe <
github.com/notifications/unsubscribe-auth/ABBHS5ZOKDJHT4O2K3E2ZMTRCUONZANCNFSM4ILE7ALQ>
.
Can do. Any notes to pass on?
The main issue I see with the output currently is that these don't resolve
-
- $ref: '#/components/parameters/include'
- $ref: '#/components/parameters/fields'
- $ref: '#/components/parameters/sort'
Will also need to bring things in line with the latest release of this lib.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#689?email_source=notifications&email_token=ABBHS56U3772YLZ5LSYEIM3RCVAZBA5CNFSM4ILE7AL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELU4KIQ#issuecomment-585745698>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABBHS54SSMIODWZSAV7QAO3RCVAZBANCNFSM4ILE7ALQ>
.
|
|
Notably /components is now in DRF....
…On Thu, Feb 13, 2020 at 8:11 AM Alan Crosswell ***@***.***> wrote:
The openapi support in upstream DRF has matured a lot so I expect there
may need to be significant changes, hopefully reducing the amount of
overridden code.
I’ve just been too busy with other work to focus at all in this.
On Thu, Feb 13, 2020 at 8:07 AM Kieran Evans ***@***.***>
wrote:
> Please do take over!
> … <#m_-3902540012596486262_m_7495308537009845494_>
> On Thu, Feb 13, 2020 at 5:30 AM Kieran Evans *@*.***> wrote: @n2ygk
> <https://github.com/n2ygk> ***@***.*** <https://github.com/n2ygk>> - Will
> you be able to address the issues raised by @sliverc
> <https://github.com/sliverc> ***@***.*** <https://github.com/sliverc>> ?
> If not, we need this, so happy to take over. — You are receiving this
> because you were mentioned. Reply to this email directly, view it on GitHub
> <#689
> <#689>?email_source=notifications&email_token=ABBHS55K5XITFUFQV3WDTGTRCUONZA5CNFSM4ILE7AL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELUHBJQ#issuecomment-585658534>,
> or unsubscribe <
> github.com/notifications/unsubscribe-auth/ABBHS5ZOKDJHT4O2K3E2ZMTRCUONZANCNFSM4ILE7ALQ>
> .
>
> Can do. Any notes to pass on?
> The main issue I see with the output currently is that these don't
> resolve -
>
> - $ref: '#/components/parameters/include'
>
> - $ref: '#/components/parameters/fields'
>
> - $ref: '#/components/parameters/sort'
>
>
> Will also need to bring things in line with the latest release of this
> lib.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#689?email_source=notifications&email_token=ABBHS56U3772YLZ5LSYEIM3RCVAZBA5CNFSM4ILE7AL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELU4KIQ#issuecomment-585745698>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABBHS54SSMIODWZSAV7QAO3RCVAZBANCNFSM4ILE7ALQ>
> .
>
|
|
Ok, great. I'll start looking into it over the next few days.
Thanks,
Kieran
…On 13/02/2020 13:14, Alan Crosswell wrote:
Notably /components is now in DRF....
On Thu, Feb 13, 2020 at 8:11 AM Alan Crosswell ***@***.***> wrote:
> The openapi support in upstream DRF has matured a lot so I expect there
> may need to be significant changes, hopefully reducing the amount of
> overridden code.
>
> I’ve just been too busy with other work to focus at all in this.
>
> On Thu, Feb 13, 2020 at 8:07 AM Kieran Evans ***@***.***>
> wrote:
>
>> Please do take over!
>> … <#m_-3902540012596486262_m_7495308537009845494_>
>> On Thu, Feb 13, 2020 at 5:30 AM Kieran Evans *@*.***> wrote: @n2ygk
>> <https://github.com/n2ygk> ***@***.*** <https://github.com/n2ygk>> - Will
>> you be able to address the issues raised by @sliverc
>> <https://github.com/sliverc> ***@***.*** <https://github.com/sliverc>> ?
>> If not, we need this, so happy to take over. — You are receiving this
>> because you were mentioned. Reply to this email directly, view it
on GitHub
>> <#689
>>
<#689>?email_source=notifications&email_token=ABBHS55K5XITFUFQV3WDTGTRCUONZA5CNFSM4ILE7AL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELUHBJQ#issuecomment-585658534>,
>> or unsubscribe <
>>
github.com/notifications/unsubscribe-auth/ABBHS5ZOKDJHT4O2K3E2ZMTRCUONZANCNFSM4ILE7ALQ>
>> .
>>
>> Can do. Any notes to pass on?
>> The main issue I see with the output currently is that these don't
>> resolve -
>>
>> - $ref: '#/components/parameters/include'
>>
>> - $ref: '#/components/parameters/fields'
>>
>> - $ref: '#/components/parameters/sort'
>>
>>
>> Will also need to bring things in line with the latest release of this
>> lib.
>>
>> —
>> You are receiving this because you were mentioned.
>> Reply to this email directly, view it on GitHub
>>
<#689?email_source=notifications&email_token=ABBHS56U3772YLZ5LSYEIM3RCVAZBA5CNFSM4ILE7AL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELU4KIQ#issuecomment-585745698>,
>> or unsubscribe
>>
<https://github.com/notifications/unsubscribe-auth/ABBHS54SSMIODWZSAV7QAO3RCVAZBANCNFSM4ILE7ALQ>
>> .
>>
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#689?email_source=notifications&email_token=AAFJJIEAZEXZWHBZGUJJ3ETRCVBVHA5CNFSM4ILE7AL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELU5AAI#issuecomment-585748481>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFJJIACFPJGGCIDO2UWEWDRCVBVHANCNFSM4ILE7ALQ>.
|
|
Closing in favor of #772 |
Fixes #604
(replaces #669)
Description of the Change
Extends DRF >= 3.10's generateschema to produce a jsonapi-formatted OAS schema document.
Checklist
CHANGELOG.mdupdated (only for user relevant changes)AUTHORS