Skip to content

Cleanup/api pass - #173

Open
Xarlos89 wants to merge 13 commits into
masterfrom
cleanup/api-pass
Open

Cleanup/api pass#173
Xarlos89 wants to merge 13 commits into
masterfrom
cleanup/api-pass

Conversation

@Xarlos89

Copy link
Copy Markdown
Collaborator

A full cleanup pass over the Flask API.
The API now uses a connection pool instead of a single psycopg connection, every route returns a correct HTTP status code, and the responses are consistent across all endpoints. Every cog was updated to the new response, and a pytest suite covering the routes was added and slammed into CI.

Fixes the long-standing jsonify(data, status_code) bug — that call serialised the status code into the body as a JSON array and always returned HTTP 200, which is why cogs were indexing responses as result[0]["logging"][2].

Closes #134

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We don't want to use the http module from stdlib here instead of a manual dict?

from http import HTTPStatus

HTTPStatus.OK == 200 # True

Comment thread src/api/api.py
Comment thread src/bot/cogs/verification/verification_dropdown.py
Comment thread src/bot/cogs/verification/verification_on_join.py
Comment thread src/api/core/db_helper.py
def get_log_setting(self, log_id):
return self._get_row(LOGGING_TABLE, log_id, "log_setting")

def get_log_settings(self):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor suggestion to avoid typos and reading too quickly and missing it...
get_log_setting and get_log_settings are very similar... perhaps a name like get_all_log_settings ?

@JefeThePug JefeThePug left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great. I've added a few comments here and there. Also some of the docker stuff is a bit over my head so I skipped it, but the python is good :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: API routes return malformed JSON — jsonify(data, status_code) should be jsonify(data), status_code

2 participants