Review Flask-Restless for approval. #485

Closed
jfinkels opened this Issue Apr 10, 2012 · 14 comments

Comments

Projects
None yet
3 participants
Contributor

jfinkels commented Apr 10, 2012

Source at https://github.com/jfinkels/flask-restless, documentation at http://readthedocs.org/docs/flask-restless. All requirements are met once I release version 0.5 tonight (which includes dual license—GNU AGPLv3 or BSD).

jfinkels referenced this issue in jfinkels/flask-restless Apr 10, 2012

Closed

Become an approved Flask extension #14

Contributor

rduplain commented Apr 10, 2012

Great -- just post here when ready for full review.

Contributor

jfinkels commented Apr 10, 2012

Okay, just uploaded version 0.5.

Contributor

alekzvik commented Apr 11, 2012

17 errors in Python 2.5:

File "flask_restless/views.py", line 775, in post
    instance = self.model(**dict([(i, params[i]) for i in props]))
TypeError: __init__() keywords must be strings
Contributor

alekzvik commented Apr 11, 2012

There is also a minor problem here:
https://github.com/jfinkels/flask-restless/blob/master/tests/test_validation.py#L248
You can't skip tests in Python < 2.7, so they are skipped, but you aren't warned about that, like all tests are OK.

Contributor

alekzvik commented Apr 11, 2012

And one more, both test from this TestCase fails on Python 2.6 and Python 2.7 if you have savalidation package installed
https://github.com/jfinkels/flask-restless/blob/master/tests/test_validation.py#L138

Contributor

jfinkels commented Apr 11, 2012

Thanks @alekzvik I'll take a look at these errors and get back to you.

Contributor

jfinkels commented Apr 11, 2012

Starting with the last comment first:

  1. I have changed the README and the skip conditions: the test requires the development version of savalidation. I have included instructions for installing it in the README.

  2. Should I do anything about this? The skips are not shown on python setup.py test, but they are shown when using run-tests.py since run-tests.py forces the use of unittest2. I don't know how to make python setup.py test use unittest2.

  3. Interesting: I am not getting these errors, when I run the unit tests on my own machine using python setup.py test, or run-tests.py, for example. Can you give me some more information about the environment in which you are finding this error? Specifically, can you tell me your Python version and how to reproduce the error?

    I have fixed the particular error in the line you quoted, but I suspect there are more lines which are affected by this issue. However, since my tests don't fail, I don't know how to check for them.

Contributor

rduplain commented Apr 11, 2012

2. The Flask extension guideline calls for python setup.py test or make test. If you can make it work through either of these, there's no problem. Otherwise, we can dig into the issue a bit and make a well-informed precedent.

Contributor

alekzvik commented Apr 11, 2012

3. You fixed that, but there is more.
https://github.com/jfinkels/flask-restless/blob/master/flask_restless/views.py#L431
Error message is the same:

File "/flask_restless/views.py", line 431, in _add_to_relation
    **dictionary)[0]
TypeError: _get_or_create() keywords must be strings

to reproduce error, try run python setup.py test in a clear virtualenv with Python 2.5.6

Contributor

alekzvik commented Apr 11, 2012

1. Poblem solved, everything works fine with dev version of savalidation

Contributor

alekzvik commented Apr 12, 2012

3. Solved.

Contributor

alekzvik commented Apr 13, 2012

I think Flask-Restless is ready to become approved. @rduplain do you agree?

Contributor

rduplain commented Apr 13, 2012

Looks great!

Contributor

alekzvik commented Apr 15, 2012

@rduplain Extension approved. Please, close the issue.

rduplain closed this Apr 15, 2012

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