-
Notifications
You must be signed in to change notification settings - Fork 4
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
Refactor following RESTful API standards fix(#22, #23, #26) #31
base: master
Are you sure you want to change the base?
Conversation
2103b6e
to
c2bc39a
Compare
Update: I removed the |
I tested this on my development machine and it seems to work if you start from scratch with an empty DB. But when I tried to apply it to a previous deployment it broke things. The table is empty and the graphs as well. I can see this on the debug.log
|
I can only guess here, but I do not think the PR was totally applied. Like you can see requests to:
in the log. This API and anything that looks like this was removed in this PR. https://github.com/Igalia/browserperfdash/pull/31/files#diff-94073f44d775e0dd3f800f89e6110efeL25 Since we have frontend changes, I think force refresh of the frontend might be required. |
Saw this still open after a couple of years. I still think the frontend somehow managed to query URLS that were removed in this PR. For example, look at this URL that was hit above:
Can we run a |
Refactors:
filter
params instead of kwargs for all query from Frontend to Backend. Eg:http://127.0.0.1:8000/dash/results?subtest=mytest&test=maintest
is how it should be - and this is how it looks now. Previouslyhttp://127.0.0.1:8000/dash/results/checjcehck/ares6
.reports/views.py
. Use inheritance whenever necessarysnake_case
in APIs. It should be alwaysnormal-case
.Further improvements:
api/v1.0/
or something. But this will break bots sending data currently.Fix: #23 #26 #22