Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#165305752 Document API using swagger #11

Merged
merged 1 commit into from
Apr 30, 2019

Conversation

Amoswachira
Copy link
Contributor

@Amoswachira Amoswachira commented Apr 29, 2019

Description

Setup Swagger and Add swagger Documentation to the already existing API endpoints.

Type of change

  • Bug fix (nonbreaking change which fixes an issue)
  • New feature (nonbreaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes

  • End to end
  • Integration

Checklist:

  • My code follows the style guide for this project
  • I have linted my code prior to submission
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Pivotal Tracker

165305752

Screenshot

screencapture-127-0-0-1-8000-api-swagger-2019-04-29-16_10_53

-install django-rest-swagger

-add swagger settings to the settings file

-add swagger url

-change the views to From apiview to genericapiviews

-update requirements.txt

-add the swagger url in the readme

[Finishes #165305752]
@@ -10,7 +10,7 @@
)


class RegistrationAPIView(APIView):
class RegistrationAPIView(GenericAPIView):

Choose a reason for hiding this comment

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

Hello @Amoswachira ,
Did this task require changing the superclass for the view from APIView to GenericAPIView?

Copy link
Contributor Author

@Amoswachira Amoswachira Apr 29, 2019

Choose a reason for hiding this comment

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

@Ogutu-Brian No, but there was an issue the parameters in the swagger documentation were not appearing when using the APIView so I change it to GenericAPIView and The parameters box appeared where a user is required to input the JSON data.

Copy link

@Ogutu-Brian Ogutu-Brian Apr 29, 2019

Choose a reason for hiding this comment

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

Does this imply that when I use APIView I will not be able to document my endpoint? We used APIViews before and we documented our endpoints with swagger. Would you check what could be amiss?

Did you get a reason as to why the APIViews could not be documented but Generic views?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ogutu-Brian This is a much efficient way as compared to what we had been doing in the past projects. when you use APIView you will have to create a separate file like swagger.py where you will have to add all the parameters manually which is tedious but when one uses GenericViews and Implements it this way. The parameters will be picked up automatically

Choose a reason for hiding this comment

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

Hello @Amoswachira ,
Imagine situations, where APIViews are more efficient to use or another developer onboarding onto the codebase, is confident with APIViews, they might feel limited or devs might be limited on the classes they can use from a framework.

If introducing the swagger.py file would solve the problem now and in future, then I suggest that we go that way. This would provide a lasting solution

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ogutu-Brian it will not limit you. After what I demonstrated to you this morning i hope everything is fine now

Choose a reason for hiding this comment

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

Yes it is

@Amoswachira Amoswachira added the reviews required PR requires reviews before merging label Apr 30, 2019
Copy link
Contributor

@Bakley Bakley left a comment

Choose a reason for hiding this comment

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

LGTM

@Amoswachira Amoswachira added finished All reviews have been implemented and removed reviews required PR requires reviews before merging labels Apr 30, 2019
@oluwagbenga-joloko oluwagbenga-joloko merged commit d7d0262 into develop Apr 30, 2019
Ogutu-Brian pushed a commit that referenced this pull request May 14, 2019
…5752

#165305752 Document API using swagger
Ogutu-Brian pushed a commit that referenced this pull request May 14, 2019
…5752

#165305752 Document API using swagger
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
finished All reviews have been implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants