Add flask-smorest dependencies and test route by nbarnabee · Pull Request #11 · wikimedia/toolhunt

nbarnabee · 2023-01-26T07:09:14Z

This commit squashes a number of smaller commits and adds the following:
flask-smorest as a dependency
A "user" model, schema, blueprint and routes for testing purposes
API documentation (auto-generated by Swagger)

To test: follow the instructions in the README to get everything running and start the db
API docs available at localhost:5000/api/documentation

To test the current route, using Postman (or equivalent), make a POST request to localhost:5000/users with the body

{
  "name":  "WhateverYouWantHere"
}

Then a GET request to the same endpoint should return the list of users.

This commit squashes a number of smaller commits and adds the following: flask-smorest as a dependency A "user" model, schema, blueprint and routes for testing purposes API documentation (auto-generated by Swagger)

nbarnabee · 2023-01-26T07:10:24Z

Oh, shoot, I should have opened this as a draft request. Feel free to check it out but I don't really want to merge until I've replaced the user test stuff with some real code.

blancadesal · 2023-01-26T08:24:29Z

Oh, shoot, I should have opened this as a draft request. Feel free to check it out but I don't really want to merge until I've replaced the user test stuff with some real code.

If this is a WIP and you want to convert it to a draft, you can still do so. There's an option for that in the right sidebar, just below the Reviewers section.

Renamed "annotations-field" to "field" Updated models Introduced schemas for Field, Tool, Task Wrote and registered /task blueprint

nbarnabee · 2023-01-31T07:52:01Z

Ok, I've now established a working route at /api/tasks and some mock data.

It does require a small amount of setup:

From the command line, docker-compose exec flask-web python manage.py create_db
Open a browser window to localhost:5000 You should receive the message "Data inserted!"
Go to localhost:5000/api/documentation and run the test to see the structure and contents of the returned data.

nbarnabee · 2023-02-01T11:31:24Z

Ok, I'm done with updates on this branch (aside from requested revisions, of course!)

It's gone well beyond its original remit, but we now have a working endpoint at api/tasks that properly distinguishes between completed and incomplete tasks. I've also upgraded SQLAlchemy to V2.

HWaruguru

Looks good. only thing is that you need to expose the database port in docker-compose so that it can be easy to view the tables in a tool like Tableplus

nbarnabee · 2023-02-01T13:09:53Z

Looks good. only thing is that you need to expose the database port in docker-compose so that it can be easy to view the tables in a tool like Tableplus

Can you open a pull request for that? I don't use any tools like that and am not sure how to configure them. If it's just a matter of exposing a port it should be an easy fix, I guess. (But nothing seems to happen easily between me and Docker.)

blancadesal · 2023-02-01T14:41:32Z

api/resources/db.py

@@ -0,0 +1,2 @@
+from flask_sqlalchemy import SQLAlchemy


For what reason(s) did you move the initialization of the db to its own file from /api/__init__py?

IIRC once I started importing it into various locations I started running into error messages; I may have gotten myself stuck in some sort of circular import loop. I'll test it out and see if I can pin down the problem.

Ah, I see. I was just caught up in the idea that I had to do all of my imports first. I can solve the problem by initializing the db and then importing the blueprints, etc. Will push a fix.

blancadesal · 2023-02-01T14:46:52Z

data/__init__.py

+from api.models import Tool, Field, Task
+from api.resources.db import db
+
+def insertData():


This could be moved to manage.py so the db could be seeded from the command line (or a script). That way, we could also remove the call to insertData from inside our index route in app.py

nit: Python variables are conventionally named using snake_case. I know you know this, but JS has a way of polluting the mind xd

Resolved with 3889f2d

blancadesal · 2023-02-01T14:47:59Z

data/fieldData.py

@@ -0,0 +1,11 @@
+fieldData = [


Having all mock data inside a single file would reduce complexity a bit

Resolved with 3889f2d

blancadesal · 2023-02-01T14:52:16Z

There's a lot going on in this PR, which is mainly my fault for not getting to it earlier. Let's do a little bit of cleanup step by step. We can start with the db stuff.

We already have the flask-migrate plugin – we can remove create_db from manage.py and instead initialize the db with poetry run flask db init. This would create a migrations folder that needs to be committed. I think there's some guidance in the flask-blueprint README.

I've also left some comments inline.

nbarnabee · 2023-02-01T16:01:02Z

There's a lot going on in this PR, which is mainly my fault for not getting to it earlier. Let's do a little bit of cleanup step by step. We can start with the db stuff.

We already have the flask-migrate plugin – we can remove create_db from manage.py and instead initialize the db with poetry run flask db init. This would create a migrations folder that needs to be committed. I think there's some guidance in the flask-blueprint README.

I've also left some comments inline.

I'd been slacking on properly implementing migrations. I've addressed the other points and will get to this one next.

The User class was for testing purposes only.

nbarnabee · 2023-02-02T06:38:56Z

So as of af7f574 I'm up to the following commands for initializing and inserting data into the db:

From the flask container, flask db upgrade which will apply the current migration and build the tables.
From the command line, docker-compose exec flask-web python manage.py insert_data

Is there a way to automate these processes? I tried various ways of inserting these commands into the Dockerfile and docker-compose, but without success.

blancadesal · 2023-02-02T06:52:38Z

Is there a way to automate these processes? I tried various ways of inserting these commands into the Dockerfile and docker-compose, but without success.

Yes – this is what the start script we have hanging around inside compose/flask can be used for. To make it work, we'd need to:

Edit the Dockerfile to copy over the start script from our directory to inside the container. To avoid some issues on Windows, we should add the line RUN sed -i 's/\r$//g' /entrypoint. The script also needs to be given execution permissions.
Edit docker-compose.yml so that it runs the start script instead of whatever command it's running right now
Edit the start script to run whatever commands we want at start-up

nbarnabee · 2023-02-02T07:33:20Z

compose/flask/start

+flask run --host=0.0.0.0 --port=5000
 #flask db upgrade
-flask run --host=0.0.0.0 --port=5000
+#python manage.py insert_data


I recalled the cv-model-serve code you'd shown me before and copied a lot of that. I was able to get things running using /start but the other commands here in the start file had no effect. (They're just commented out here because they weren't doing anything and I wanted to commit a "working" version.)

(I was intending to highlight multiple lines of code here but obviously that didn't work out. I'm referring to the contents of compose/flask/start)

* Add /api/contribution endpoint * Add route for getting leaderboard data

Add flask-smorest dependencies and test route

f8d7a75

This commit squashes a number of smaller commits and adds the following: flask-smorest as a dependency A "user" model, schema, blueprint and routes for testing purposes API documentation (auto-generated by Swagger)

nbarnabee requested review from HWaruguru and blancadesal January 26, 2023 07:10

nbarnabee marked this pull request as draft January 26, 2023 09:43

nbarnabee added 4 commits January 30, 2023 11:43

Add Toolhunt models v1

470d74b

Establish GET route at /api/task

3653cdf

Renamed "annotations-field" to "field" Updated models Introduced schemas for Field, Tool, Task Wrote and registered /task blueprint

Establish automatic insertion route

89409d7

Update README

73afacb

nbarnabee marked this pull request as ready for review January 31, 2023 07:52

nbarnabee requested a review from Damilare1 January 31, 2023 07:52

nbarnabee added 3 commits February 1, 2023 10:05

Clean up code & expand mock data

cc24432

Migrate to SQLAlchemy 2

6d7cc7e

Update task schema, blp and query

16f2aa6

HWaruguru requested changes Feb 1, 2023

View reviewed changes

HWaruguru approved these changes Feb 1, 2023

View reviewed changes

nbarnabee mentioned this pull request Feb 1, 2023

Add contributions route #12

Closed

blancadesal reviewed Feb 1, 2023

View reviewed changes

nbarnabee added 2 commits February 1, 2023 16:29

Return db initialization to api/__init__.py

f198251

Move db population function into manage.py

3889f2d

nbarnabee added 2 commits February 2, 2023 07:00

Remove User model, schema, blp & refs

703a111

The User class was for testing purposes only.

Add db migration via flask-migrate & alembic

931f472

nbarnabee requested a review from blancadesal February 2, 2023 06:02

Update README

af7f574

Take start file into partial use

9315665

nbarnabee commented Feb 2, 2023

View reviewed changes

nbarnabee mentioned this pull request Feb 2, 2023

Add contributions route #14

Merged

Add contributions route (#14)

fc6c7eb

* Add /api/contribution endpoint * Add route for getting leaderboard data

Damilare1 approved these changes Feb 7, 2023

View reviewed changes

nbarnabee merged commit 24b89c4 into main Feb 7, 2023

nbarnabee deleted the add_smorest_and_routes branch February 7, 2023 11:21

Conversation

nbarnabee commented Jan 26, 2023

Uh oh!

nbarnabee commented Jan 26, 2023

Uh oh!

blancadesal commented Jan 26, 2023

Uh oh!

nbarnabee commented Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nbarnabee commented Feb 1, 2023

Uh oh!

HWaruguru left a comment

Choose a reason for hiding this comment

Uh oh!

nbarnabee commented Feb 1, 2023

Uh oh!

blancadesal Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

nbarnabee Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

nbarnabee Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

nbarnabee Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

blancadesal Feb 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nbarnabee Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

blancadesal Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

nbarnabee Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

blancadesal commented Feb 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nbarnabee commented Feb 1, 2023

Uh oh!

nbarnabee commented Feb 2, 2023

Uh oh!

blancadesal commented Feb 2, 2023

Uh oh!

nbarnabee Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nbarnabee commented Jan 31, 2023 •

edited

Loading

blancadesal Feb 1, 2023 •

edited

Loading

blancadesal commented Feb 1, 2023 •

edited

Loading

nbarnabee Feb 2, 2023 •

edited

Loading