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

Make API discoverable from the root #705

Merged
merged 6 commits into from
Aug 7, 2018
Merged

Make API discoverable from the root #705

merged 6 commits into from
Aug 7, 2018

Conversation

fpagnoux
Copy link
Member

@fpagnoux fpagnoux commented Aug 3, 2018

Connected to #615

  • On the Web API, expose a welcome message (with a 300 code) on / instead of a 404 error.

For instance, curl -i localhost:5000 gives:

HTTP/1.1 300 MULTIPLE CHOICES
(...)

{
  "welcome": "This is the root of an OpenFisca Web API. To learn how to use it, check the general documentation (https://openfisca.org/doc/api) and the OpenAPI specification of this instance (http://localhost:5000/spec)."
}
  • This message can be customized:

If the Web API is started with openfisca serve -p 3000 --welcome-message "Welcome to the OpenFisca-France Web API. To learn how to use it, check our interactive swagger documentation: https://fr.openfisca.org/legislation/swagger."

Then curl -i localhost:5000 gives:

HTTP/1.1 300 MULTIPLE CHOICES
(...)

{
  "welcome": "Welcome to the OpenFisca-France Web API. To learn how to use it, check our interactive swagger documenation: https://fr.openfisca.org/legislation/swagger"
}

(Like other configuration variables, this custom message can also be defined in a configuration file. Check the openfisca serve documentation)

Copy link
Contributor

@Anna-Livia Anna-Livia left a comment

Choose a reason for hiding this comment

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

Great work :)
I have one question and then it is good to go ^^

@@ -81,6 +98,8 @@ def get_variable(id):

@app.route('/spec')
def get_spec():
if data['host'] is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

This condition is checked in def resolve_host. Isn't it redundant to have it here also ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The spec route can be called before the / one. In that case, data['host'] is still None when we get here, so we need to resolve the host.

Copy link
Member Author

Choose a reason for hiding this comment

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

However, I was just told that re-using the host from one request to another was risky (one local request, and everyone is seeing a localhost:3000 as a host) and not very stateless, so we need a different approach 😞

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, adapted the code :)

@fpagnoux fpagnoux requested a review from Anna-Livia August 6, 2018 23:11
@fpagnoux fpagnoux merged commit 791b8ae into master Aug 7, 2018
@fpagnoux fpagnoux deleted the api-discoverable branch August 7, 2018 22:55
@bonjourmauko bonjourmauko added the kind:feat A feature request, a feature deprecation label Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feat A feature request, a feature deprecation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants