Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

Fix OpenAPI schema for async responses #3544

Merged
merged 1 commit into from Jul 8, 2018
Merged

Conversation

werwty
Copy link
Contributor

@werwty werwty commented Jul 6, 2018

@pep8speaks
Copy link

pep8speaks commented Jul 6, 2018

Hello @werwty! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on July 07, 2018 at 19:03 Hours UTC

@codecov
Copy link

codecov bot commented Jul 6, 2018

Codecov Report

Merging #3544 into master will increase coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #3544     +/-   ##
=========================================
+ Coverage   57.34%   57.44%   +0.1%     
=========================================
  Files          59       59             
  Lines        2459     2465      +6     
=========================================
+ Hits         1410     1416      +6     
  Misses       1049     1049
Impacted Files Coverage Δ
pulpcore/pulpcore/app/serializers/__init__.py 100% <ø> (ø) ⬆️
pulpcore/pulpcore/app/serializers/base.py 55.47% <100%> (+0.93%) ⬆️
pulpcore/pulpcore/app/viewsets/repository.py 71.51% <100%> (+0.18%) ⬆️
pulpcore/pulpcore/app/viewsets/base.py 75.8% <100%> (+0.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 408f41d...4b98650. Read the comment docs.

Copy link
Contributor

@daviddavis daviddavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Do plugins and plugin_template need this update too?

@dralley
Copy link
Contributor

dralley commented Jul 6, 2018

I can copy them over to plugin_template since I've already got a PR in progress there

@@ -229,6 +237,9 @@ def destroy(self, request, repository_pk, number):
)
return OperationPostponedResponse(async_result, request)

@swagger_auto_schema(operation_description="Trigger an asynchronous task to create "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good if the other descriptions would have the same level of detail w/r/t what they are updating or deleting (a Repository Version).

Otherwise, LGTM

@@ -59,6 +59,8 @@ class RepositoryViewSet(NamedModelViewSet,
pagination_class = NamePagination
filter_class = RepositoryFilter

@swagger_auto_schema(operation_description="Trigger an asynchronous update task",
responses={202: AsnycOperationResponseSerializer})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same applies here

@werwty
Copy link
Contributor Author

werwty commented Jul 7, 2018

I will open a PR to pulp_file, and pulp_ansible with these changes.
@dralley if you don't mind I can open a separate PR to plugin_template to address this, since there should be a few comments explaining to the plugin writer why this is necessary.

@werwty
Copy link
Contributor Author

werwty commented Jul 8, 2018

Plugin PRs:
pulp_ansible: pulp/pulp_ansible#39
pulp_file: pulp/pulp_file#99
pulp_python: pulp/pulp_python#190
plugin_template: pulp/plugin_template#11

@werwty werwty merged commit 055c3da into pulp:master Jul 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants