Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[1.0.01b] primary_key argument in create_api_blueprint() #540

Closed
yhosun opened this issue May 30, 2016 · 8 comments
Closed

[1.0.01b] primary_key argument in create_api_blueprint() #540

yhosun opened this issue May 30, 2016 · 8 comments
Labels

Comments

@yhosun
Copy link

yhosun commented May 30, 2016

In 1.0.01b, even if primary_key argument is provided, create_api_blueprint() throws:

flask_restless.manager.IllegalArgumentError: Provided model must have an `id` attribute

I think this validation also needs to consider the value of primary_key:
https://github.com/jfinkels/flask-restless/blob/master/flask_restless/manager.py#L597-L599


I also have a question:
http://flask-restless.readthedocs.io/en/1.0.0b1/basicusage.html?highlight=sqlalchemy.Integer

each model must have a primary key column named id of type sqlalchemy.Integer or type sqlalchemy.Unicode.

Do you also support a primary key column of sqlalchemy.BigInteger? In 0.17.0, I did not notice any issue with it.

@yhosun yhosun changed the title [1.0.01b] IllegalArgumentError: Provided model must have an id attribute [1.0.01b] primary_key argument in create_api_blueprint() May 30, 2016
@yhosun
Copy link
Author

yhosun commented May 30, 2016

In the following code, it is also assumed that id attribute always exists:
https://github.com/jfinkels/flask-restless/blob/master/flask_restless/serialization/serializers.py#L354

@jfinkels
Copy link
Owner

jfinkels commented Jun 6, 2016

As for the primary_key issue, yes I believe this is a bug (in both the code and the documentation). Can you draw up some tests for it?

As for the BigInteger issue, can you please open a new issue for it? Thank you.

@jfinkels jfinkels added the bug label Jun 6, 2016
@talatbaig
Copy link

Hi

Any update on the issue requiring an id, even if a primary key column is present?

@yhosun
Copy link
Author

yhosun commented Jun 16, 2016

I have not had a chance to create a testcase, but if I modify an example in the doc to reproduce:
http://flask-restless.readthedocs.io/en/1.0.0b1/basicusage.html

class Person(Base):
    person_id = Column(Integer, primary_key=True)  # person_id instead of id
blueprint = manager.create_api_blueprint('person', Person, methods=methods)

And then, if there is a GET request, the error should be thrown.

@tanj
Copy link

tanj commented Jun 27, 2016

I have got myself running with the following patch. I haven't tested any further than it seems to work for me.

https://github.com/tanj/flask-restless/commit/a0880befbbacf49efc59cd7ecea02e6a944c2f75

@smitty1eGH
Copy link

After doing a pip install git+https checkout of the code, I was able to at least boot my flask server with lines 598-600 commented out.
There is this example of automating detection of the primary key, since we're asserting SQLAlchemy:
http://stackoverflow.com/questions/6745189/how-do-i-get-the-name-of-an-sqlalchemy-objects-primary-key

@jfinkels
Copy link
Owner

jfinkels commented Jul 5, 2016

@yhosun Thanks for providing a test, I'll use that example as the basis for a unit test.

@tanj Thanks for the suggested patch, I'll use that as the basis for a more complete solution.

@smitty1eGH The top answer is mine from when I was trying to figure out how to find primary keys programmatically :)

This should be fixable by using the primary_key_for helper function.

@jfinkels
Copy link
Owner

Fixed by pull request #581. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants