Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Flask REST API #16

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Flask REST API #16

wants to merge 12 commits into from

Conversation

bmenn
Copy link
Member

@bmenn bmenn commented Apr 19, 2017

Creating this in place of #15 so more people can easily contribute.

@ghost ghost assigned bmenn Apr 19, 2017
@ghost ghost added the board:in-progress label Apr 19, 2017
@bmenn bmenn assigned kaushik316 and unassigned bmenn Apr 19, 2017
@bmenn bmenn mentioned this pull request Apr 19, 2017
@bmenn
Copy link
Member Author

bmenn commented May 3, 2017

@kaushik316

Good work, I'll give this a more thorough review later.

@kaushik316
Copy link
Collaborator

FYI the free text search only works for the backpagecontent table. Ill have to figure out how to join tables so you can search with phone numbers, email, etc.

@bmenn
Copy link
Member Author

bmenn commented May 4, 2017

Did another look over. Besides your comments about adding more functionality to the search REST API, it looks good.

Nice job! Let me know if you need help with the additional functionality.

@kaushik316
Copy link
Collaborator

Have added the code that @bmenn and I worked on last night. Now we should probably figure out what endpoints would make the most sense and what other information specifically we're looking for from the API. For example, here's a route that gets post titles based on post id:

@app.route('/api/backpage/<int:backpagepost_id>/title', methods=['GET'])
def get_title_withID(backpagepost_id):
    numbers = (Backpagephone.query.filter_by(backpagepostid=backpagepost_id).all())

    return jsonify({'numbers': [content.title for n in numbers for content in n.backpagepost.content.all()]})

I think we discussed adding an endpoint with a free text search parameter for words in the title of a post. Were there any others in particular that come to mind?

Copy link
Contributor

@dlrobertson dlrobertson 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!

Can the CREATE, *.pyc, and .DS_Store files be removed? Maybe those should be added to the `.gitignore.

Another design consideration to think about. Do we want to add pagination to the endpoints that could return a lot of ouput? E.g api/backpage/phone/ could return a lot of data. Perhaps instead of

{
    data: [
        {
            id: <id>,
            number: "<number>"
        },
        ...
        all the data
    ]
}

Maybe we should have something like api/backpage/phone/offset=i

{
    data: [
        {
            id: <id>,
            number: "<number>"
        },
        ...
        i through i + n
    ]
    length: n
}

I don't know a lot about flask or this particular implementation, so let me know if I'm way off. Just a thought

output = str(e)
is_database_working = False

print (output)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary? or just a debug statement?

Copy link
Collaborator

@kaushik316 kaushik316 Jun 22, 2017

Choose a reason for hiding this comment

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

Will add the CREATE, .pyc and .DS_Store files to the gitignore on the next commit.

The lines you mentioned and the entire healthcheck method can be deleted, I just added them to make sure the database was working properly on my machine (for clarity, all this code can be removed):

health = HealthCheck(app, "/healthcheck")

def health_database_status():
    is_database_working = True
    output = 'database is ok'

    try:
        session = db.session()
        session.execute('SELECT * from backpageemail')
    except Exception as e:
        output = str(e)
        is_database_working = False

    print (output)

I'm not a Flask wizard either so I'll look into the pagination and try to implement that if that's possible in Flask. Currently you can pass a specific number or id as a parameter to get a subset of results.

@dlrobertson
Copy link
Contributor

Were there any others in particular that come to mind?

Most of the queries I can remember are there. There should probably be a endpoint for the page table, since that is the "base" table. There should also be an endpoint that allows a user to get the raw html content of the page. This could be either the endpoint for the page table or the content table. I'm not sure what the best way to deliver that would be? Just a massive string containing the html?

@bmenn
Copy link
Member Author

bmenn commented Jun 23, 2017

@kaushik316

I'll take look at the changes this weekend. Busy week at work. I think you might be able to do pagination with combination of SQLAlchemy and Flask. Examples:

That last link is an amazing resource on Flask.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants