rdegges / flask-dynamo

DynamoDB integration for Flask.
http://flask-dynamo.readthedocs.org/en/latest/
The Unlicense
141 stars 36 forks source link

Creation error handling #15

Closed Seraf closed 8 years ago

Seraf commented 8 years ago

Hello,

I added few things in this PR:

with application.app_context():
     application.config['DYNAMODB'].create_all()

This created my tables the first time, then raise an error the next times. I need to let the creation occurs at each launch if they aren't exist. The problem with the actual code is if I want to modify the throughput of my table, there's no automagical modification and if I add a table, the first exception will exit the loop without creating the other tables.

So I modified the behavior of the create function : it will try to create a new table. If it fails, it will try an update on it. If the fields remains the same, Dynamodb API will raise a 400 error which is catched and add the table name to a list of failed tables. It will continue to create/update other tables and will raise a custom exception named : DynamoTableError at the end of the loop if there's failed tables.

On my main code I can now use :

with application.app_context():
    try:
        application.config['DYNAMODB'].create_all()
    except DynamodbTableError as e:
        application.logger.info(e)

What do you think about it ?

Thanks for your work

rdegges commented 8 years ago

Thanks for the awesome PR!

So, some feedback for this: I'm not a fan of overloading the create_all method, to have it do updating as well. My reasoning is that it's sorta useful to just have a creation thing to create basic tables and ensure they exist, as well as have a way to destroy all tables (the destroy_all method).

What if you modified your PR to do the following?

What do you think?

Seraf commented 8 years ago

Hello @rdegges,

Thanks for the answer. I understand your point of view and I think you are right. I was just testing/playing with Dynamodb, and have no more time to work it as it was just a proof of concept. Just to let you know, I finished to play with flywheel library which does more what I was looking for.

Anyway, thanks for your work, working on this PR let me understand how the Dynamodb was working :)

Thanks

rdegges commented 8 years ago

Thank you, that makes sense. I appreciate the contribution regardless! I'll get this in there myself when I get some time. =)