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

[openapi] API definition using Open API v3 #122

Closed
wants to merge 16 commits into from

Conversation

dellagustin
Copy link
Contributor

@dellagustin dellagustin commented Jun 22, 2018

  • Client Parametrization
  • Authentication API
  • Directory API
  • Suggestions API
  • Device API
  • Subscriptions API
  • Episode Actions API
  • Podcast Lists API
  • Settings API
  • Favorites API
  • Device Synchronization API
  • Add test for OpenAPI definition validity
  • Fix failing test
  • Change contact email
  • Expose API in the website
  • Add link to the API documentation to the OpenAPI definition
  • Remove .pytest_cache

At the moment, response schemas are out of scope.

@stefankoegl
Copy link
Member

I've read up on the OpenAPI initiative, and it looks quite interesting.

Are you creating the definition with a specific client / use case in mind?

How is one supposed to publish this definition?

@dellagustin
Copy link
Contributor Author

Hi Stefan,

I plan to use the API in my chrome extension to subscribe and listen to podcasts, it is called podStation:
https://github.com/podStation/podStation
It is currently not available in the chrome web store due to some publishing issues, but It will soon be available again.

I belive it can be used to generate consumer code in many different programming languages.

If I undestood well you can publish it in your own website using the swagger-ui: https://github.com/swagger-api/swagger-ui

@dellagustin
Copy link
Contributor Author

I finished defining all the APIs, I'm taking a look on how to publish it now

@stefankoegl
Copy link
Member

There's a openapi-spec-validator. It might make sense to include the validation in the tests, to make sure we always have a valid spec file.

@dellagustin
Copy link
Contributor Author

@stefankoegl I'm trying to run the tests with cmd pytest --cov=mygpo/ --cov-branch, but it seems to get stuck at mygpo/administration/tests.py

I'm running postgres using a slightly modified version of your docker-compose file from the docker branch which exposes the port 5432.

Do you have any idea what it could be?

@stefankoegl
Copy link
Member

Do you know where exactly it gets stuck? When you press Ctrl+C when the test hangs, it should give you a stack trace.

@dellagustin
Copy link
Contributor Author

That actually helped, I run it with a --fulltrace and Ctrl+C showed it was stuck in some retry loop related to ampq and celery, so I realised I should be runing redis as well.
That is not in the list of dependencies though (http://gpoddernet.readthedocs.io/en/latest/dev/installation.html).
Although seeing the docker-compose file and the travis.yml, I should have figured. Now it is running.
Thanks!

@stefankoegl
Copy link
Member

I noticed that you added two files in .pytest_cache/, which was probably intentional. Could you please remove them again, and add the folder to .gitignore ?

@dellagustin
Copy link
Contributor Author

Scheiße! That is what you get when trying to finish stuff late at night 😢 (and trying to be a purist)

I actually have added .pytest_cache to .gitignore in a separate branch and forgot to send the PR yesterday. I sent it now (#138)

I was trying to manually avoid adding this folder until the PR was merged, but as you can see I failed at that.

I'll fix it.

@dellagustin
Copy link
Contributor Author

Hi @stefankoegl , I implemented a prototype on how to expose the openapi definition on the website using swagger-ui in this branch: https://github.com/podStation/mygpo/tree/openapi_expose
(I'm not very confident on the code, that is why I branched it)

Here is how it looks like:
image

you can access it at /api

I'm not sure about embbeding it in the website this way as it limits the size and messes up a little bit with the style, perhaps it would be better to simply host the swagger-ui without wrapping it into the website ui.

I'm not very happy with loading the dependencies direcly from unpkg, but also copying the resources to /static/js and adding them to git also is not ideal. It would be interesting to install the front end dependencies on travis just as the python dependencies, but that is a development of its own.

Let me know your thoughts.

PS: my plan is to keep the lower menu pointing to readthedocs and in there point to the swagger-ui, as some devs may be still used to the readthedocs thing, and also the api definition is not 100% complete yet.

@dellagustin
Copy link
Contributor Author

@stefankoegl Just a ping to check if you saw my last comment

@stefankoegl
Copy link
Member

My understanding was that we would publish the openapi.yaml file at some public URL for the clients to consume. I didn't know that it would involve including some third party software within the gpodder.net interface.

If this is necessary, it might be better to keep this as a standalone application.

What do you think?

@dellagustin
Copy link
Contributor Author

Hi Stefan,

I think it makes sense to keep it as a standalone app as you suggested.
Including it in the UI looks a bit strange, also it is a developer tool.

What do you think of the approach I applied at commit 603cfd3 to serve the api definition?

If that is ok I can integrate that commit in my branch for this PR and point to it at the API documentation, then we can close this PR and discuss how to host the swagger UI in a different thread.

@SiqingYu SiqingYu self-requested a review February 7, 2020 02:07
@SiqingYu SiqingYu self-assigned this Feb 7, 2020
@SiqingYu
Copy link
Contributor

SiqingYu commented Feb 7, 2020

I'm assigned this issue and currently triaging. If you have any updates, please let me know.

@SiqingYu
Copy link
Contributor

Hi @dellagustin. I don't know how to generate the openapi.yaml file. Could you please tell me how to generate the OpenAPI specification automatically for our codebase? It's hard to write the specs manually for numerous APIs. Thanks for your help. 😄

@SiqingYu
Copy link
Contributor

I think https://github.com/OpenAPITools/openapi-generator is used to generate code from the YAML specs, but what about the reverse process (generate YAML from existing code)?

@SiqingYu SiqingYu mentioned this pull request Feb 10, 2020
1 task
@SiqingYu
Copy link
Contributor

Because the branch has been stale for some time. I rebased and opened a new pull request #361

If you have any update, please comment under the new pull request.

@SiqingYu SiqingYu closed this Feb 10, 2020
@dellagustin
Copy link
Contributor Author

Hi @SiqingYu,

First of all, thank you for picking up where I left! It was so long ago that I don't even know where I stopped.

I wrote the OpenAPI specification manually as I don't know how to generate it from code, but that is a valid question.
If I find examples for that in python I will update you in #361 , but I can't promise anything.
Good luck!
If you have any questions about my work there, just let me know, I will try to answer as soon as possible.

@SiqingYu
Copy link
Contributor

@dellagustin

Thanks for your reply. I will carry on with your works.

@SiqingYu SiqingYu mentioned this pull request Feb 16, 2020
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Improvements
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

3 participants