Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Review Flask-Restless for approval. #485

Closed
jfinkels opened this Issue · 14 comments

3 participants

@jfinkels

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 jfinkels referenced this issue in jfinkels/flask-restless
Closed

Become an approved Flask extension #14

@rduplain
Collaborator

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

@jfinkels

Okay, just uploaded version 0.5.

@alekzvik

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
@alekzvik

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.

@alekzvik

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

@jfinkels

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

@jfinkels

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.

@rduplain
Collaborator

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.

@alekzvik

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

@alekzvik

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

@alekzvik

3. Solved.

@alekzvik

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

@rduplain
Collaborator

Looks great!

@alekzvik

@rduplain Extension approved. Please, close the issue.

@rduplain rduplain closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.