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

#157 Listed available routes #174

Merged
merged 1 commit into from Oct 21, 2017

Conversation

Projects
None yet
3 participants
@vessemer
Copy link
Contributor

vessemer commented Oct 19, 2017

This http://0.0.0.0:8000/ route to a page listing all the available routes.

Description

http://0.0.0.0:8000/ redirect to /api/routes which in turn will return the dictionary of available links.

Reference to official issue

This addresses #157.

Motivation and Context

Currently, navigating to http://0.0.0.0:8000/ returns a 404 error.

Screenshots:

The API and JSON response:
screenshot from 2017-10-19 16-46-23
screenshot from 2017-10-19 16-47-42

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
@isms

This comment has been minimized.

Copy link
Contributor

isms commented Oct 19, 2017

@vessemer sorry, may have missed some of the context here, but why can't people just look at the source code to figure out the routes?

@vessemer

This comment has been minimized.

Copy link
Contributor

vessemer commented Oct 19, 2017

@isms, I guess, that the point of the issue #157 is more about to get rid of an error 404, while navigating to http://0.0.0.0:8000/. List of available routes in this case may serve as a fancy hint for the user.

@isms

This comment has been minimized.

Copy link
Contributor

isms commented Oct 19, 2017

How about just a redirect from / to /api since that shows all the available endpoints? Scraping the source code for url patterns doesn't seem like a great long term answer.

@vessemer vessemer force-pushed the vessemer:157_home_route branch from bb088ca to c913628 Oct 20, 2017

@vessemer

This comment has been minimized.

Copy link
Contributor

vessemer commented Oct 20, 2017

@isms, I have to agree with you, it seems that it fits the requirements of the issue #157.
@reubano can you take a look, please?

@vessemer vessemer force-pushed the vessemer:157_home_route branch from c913628 to 1bbab97 Oct 20, 2017

@isms
Copy link
Contributor

isms left a comment

Request you remove everything except a redirect from / to /api.

@vessemer vessemer force-pushed the vessemer:157_home_route branch 2 times, most recently from 9257dce to 172688c Oct 20, 2017

@vessemer

This comment has been minimized.

Copy link
Contributor

vessemer commented Oct 20, 2017

@isms, done: all squashed and rebased.

@vessemer vessemer force-pushed the vessemer:157_home_route branch from 172688c to 82c3cc0 Oct 20, 2017

@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Oct 21, 2017

Merging, but I think I would have liked to have seen the "urlconf" instead of the raw /api in the redirect ;)

@lamby lamby merged commit 3d4a06d into drivendataorg:master Oct 21, 2017

2 checks passed

concept-to-clinic/cla @vessemer has signed the CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment