pyeve / eve-sqlalchemy

SQLAlchemy data layer for Eve-powered RESTful APIs
http://eve-sqlalchemy.readthedocs.io
Other
234 stars 70 forks source link

Eve 0.6 compatibility #91

Closed dkellner closed 7 years ago

dkellner commented 8 years ago

I'm currently working on compatibility for Eve 0.6 in https://github.com/dkellner/eve-sqlalchemy/tree/eve-0.6, but there is still some work to do and a lot of rough edges to clean. There will be a pull request as soon as it is ready.

There are at least two noticeable changes:

  1. Refactoring of Eve integration tests so we don't copy them anymore (if not necessary), but use multiple inheritance to run all of Eve method tests by default (concerns get.py, post.py, put.py, patch.py and delete.py). This involved mimicing the resources (models) Eve uses for its integration tests and not inventing new ones. This should make it easier to keep up with Eve development in the future.
  2. Handling of related IDs and embedded objects with just one field in the payload, as Eve does.
jeroenes commented 8 years ago

Great, thanks a lot! If you need help testing or so please let me/us know. Jeroen

dkellner commented 8 years ago

@jeroenes: I've created the pull request just now. Could you test the code with your application?

Most things should work by now, I've skipped a few tests with "not implemented yet". These are for features which didn't work in former versions as well. A further roadmap would include changing some tests directly in Eve to remove the need of certain overrides in Eve-SQLAlchemy. And little refactoring here and there... :)

botzill commented 8 years ago

Hi.

Thx a lot for porting this to eve 0.6, it's great to keep things in sync.

I tried the code example from the branch eve-0.6 but I still get the error like:

Traceback (most recent call last):
  File "sqla_example.py", line 6, in 
    app = Eve(validator=ValidatorSQL, data=SQL)
  File "/Users/chirgeo/.virtualenvs/eve_sqla/lib/python2.7/site-packages/eve/flaskapp.py", line 171, in __init__
    self.register_resource(resource, settings)
  File "/Users/chirgeo/.virtualenvs/eve_sqla/lib/python2.7/site-packages/eve/flaskapp.py", line 809, in register_resource
    self._validate_resource_settings(resource, settings)
  File "/Users/chirgeo/.virtualenvs/eve_sqla/lib/python2.7/site-packages/eve/flaskapp.py", line 321, in _validate_resource_settings
    self.validate_schema(resource, settings['schema'])
  File "/Users/chirgeo/.virtualenvs/eve_sqla/lib/python2.7/site-packages/eve/flaskapp.py", line 393, in validate_schema
    "'%s'" % id_field)
eve.exceptions.SchemaException: 'unique' key is mandatory for id field '_id'

I'm not sure if this should be fixed in the changes you made, but I want to help with testing.

Here are the pip freeze result:

Cerberus==0.9.2
Eve==0.6.1
Events==0.2.1
Flask==0.10.1
Flask-PyMongo==0.3.1
Flask-SQLAlchemy==2.1
itsdangerous==0.24
Jinja2==2.8
MarkupSafe==0.23
pymongo==2.9.1
simplejson==3.8.1
SQLAlchemy==1.0.10
Werkzeug==0.10.4
wheel==0.24.0

Thx!

dkellner commented 8 years ago

@botzill: Thank you for testing! Could you check if your _id column has unique=True or primary_key=True? If yes, {'unique': True} should be set automatically in the schema definition of your resource and therefor satisfy this check done by Eve.

botzill commented 8 years ago

I'm using default example code from example folder

So, by default that _id field have unique: False. If I set to smth like this it works:

DOMAIN['people']['schema']['_id'].update({
    'unique': True
})
DOMAIN['invoices']['schema']['_id'].update({
    'unique': True
})

Is there any way we can set by default that _id to unique?

botzill commented 8 years ago

One more scenario that works different from the previous version. If we have a foreign key column like:

people_id = Column(Integer, ForeignKey('people.id'))

then it is not added by default to schema when we do:

Invoices._eve_schema['invoices']

I know that we can add it manually, but I like more the auto version(and remove if not needed). Note that here I'm specifically using code snippets from example project for easier referencing.

jeroenes commented 8 years ago

@dkellner

Thanks a lot for updating the code! The trivial example works, and also my large(r) project I am working on. Only effort I had to put in was to figure out how to pip a pull request :-)

And BTW, thanks to all developers for Eve-SQLAlchemy. Really awesome way to write RESTful APIs, directly against a solid database structure!

Now up to implementing the Eve 0.6 features into my code...

Cheers, Jeroen

dkellner commented 8 years ago

@botzill

(1) I completely forgot about the examples - shame on me. They're updated now and work again. The reason for _id not being unique was it's been a hybrid property for backward compatibility. I've removed that and configured the id field properly.

(2) You have to define a relationship to the other table as well in order to get a field in your schema. The reason is that by default, Eve uses just one field in the payload for both id and embedded object (eg. 'person'). In former versions of Eve-SQLAlchemy, this was different and two fields were used (e.g. 'person_id' and 'person'). I know this is a breaking change for some, but imo it is desirable to mimic Eve as close as possible regarding the actual interface, making changes only to the way data is stored.

botzill commented 8 years ago

Hi.

Thx for the update. I'm still confused about the 'person' field not being include by default. You say:

"You have to define a relationship to the other table as well in order to get a field in your schema",

but we have a relation defined via:

people = relationship(People, uselist=False)

so, why it's not included in the schema definition by default?

Sorry I'm not so familiar with eve or eve-sqlachemy, so my question may be stupid.

dkellner commented 8 years ago

@botzill: There are no stupid questions, so keep on asking. Maybe we really have an issue here that is worth to be discovered. I'm going to describe the steps I've tried to reproduce your problem:

  1. (Make sure to use most recent code from eve-0.6 branch, examples got changed recently.)

    $ cd eve-sqlalchemy
  2. Setup virtual environment:

    $ virtualenv .
    $ source bin/activate
    $ python setup.py install
  3. Run the example server:

    $ python examples/sqla_example.py
    -> Running on http://127.0.0.1:5000/
  4. A first GET on /invoices yields no results, as there are no invoices created by the test script:

    $ curl -X GET -i http://localhost:5000/invoices
  5. Create an invoice referencing the person with id 13:

    $ curl -X POST -i -d '{"number": 42, "people": 13}' -H 'Content-Type: application/json' http://localhost:5000/invoices
  6. Repeat the GET request:

    $ curl -X GET -i http://localhost:5000/invoices

Now I'm actually getting one invoice item with "people": 13 in the payload. Is this working for you, too?

Additionally you can check the created schema by adding

from pprint import pprint
pprint(Invoice._eve_schema['invoices'])

(or a pdb breakpoint) at the end of examples/settings.py and verify that people is there.

botzill commented 8 years ago

Actually you a right, it works the way you described. I was confused by the fact that if people_id is null then field people was not retuned by the GET api call. I did insert few entries in Invoices table directly via the DB driver and people_id was null.

Now all works OK, thx a lot for this migration. I will keep testing it with a bigger project I have and see if all works.

Good job!

mandarvaze commented 8 years ago

I'm now using Eve-SQLAlchemy version 0.4.1 with Eve 0.6.1 Seems to be working fine.

When can we expect official update on pypi where Eve-SQLAlchemy officially works with Eve 0.6 ?

dkellner commented 8 years ago

I found a bug regarding relation fields in where clauses and updated the pull request accordingly.

When can we expect official update on pypi where Eve-SQLAlchemy officially works with Eve 0.6 ?

@amleczko: Did you have time already to look at my changes and decide whether they can be merged into a new release?

leedhamj commented 8 years ago

Hi,

Just following up on this somewhat old thread, but I can't find a relevant more recent one.

Just bumped into SOFT_DELETE not being in <0.6. How close is a 0.6 release version?

John

dkellner commented 8 years ago

@leedhamj: That's a good question. There was no "official" activity in this project since December last year. I am currently travelling and don't have time to look into this, but will do so when I get back home. It would be nice to get at least some reaction by a maintainer/RedTurtle.

tejovanthn commented 8 years ago

Hey, any updates on this repo coming up? A bunch of stuff waiting to be closed I guess.

dkellner commented 7 years ago

@tejovanthn: Unfortunately not. The maintainer(s) of this project still don't care about contributions, so the only solutions seems to be a proper maintained fork including own packages on pypi, taking care of the community (i.e. at least replying to peoples' issues and PRs...). I don't have time to take care of this at the moment. If somebody else is going to do this, I'd be happy to contribute.

tejovanthn commented 7 years ago

@dkellner, I'm interested in taking this forward, only because I'm all-in for postgres/sqlalchemy than for mongo. :stuck_out_tongue: Will fork your repo, and see how I can contribute. Thanks for making the port for 0.6, this is a great starting point for me. :smile:

joystein commented 7 years ago

Maybe try to coordinate with @nicolaiarocci of the parent project? Ideally there would be an umbrella github organization for eve projects that could ensure continuity (handling pip credentials and access to authoritative repo) in cases such as these where the maintainers apparently abandons the project.

I might also be interested in contributing btw.

amleczko commented 7 years ago

Hi guys, sorry for not responding previously. I'm not currently able to maintain the project and I was the only one in RedTurtle working on it. Forking it is an option you are always free to do it but IMO is not the best. Having a separate organization would be better. If smb is interested in maintenance - let me know.

joystein commented 7 years ago

@amleczko : good to hear from you :-)

I'm just speaking as someone who have found your work very useful - but, would you have the time to try to work together with @nicolaiarocci (if there is any interest from his side) to setup a github organization where eve-sqlalchemy could live on with new maintainers? I believe several has already shown at least some interest in helping out.

The prospect of seeing an updated package on pip in the not so distant future would mean a lot to me, and i guess, to quite a few others as well.

nicolaiarocci commented 7 years ago

Hello everybody. I'm willing to help if there's anyone stepping up to become the next eve-sqlalchemy maintainer. First goal of course would be to bring it up to Eve v0.6 or even v0.7 (which is going to be released soon).

I am not 100% convinced on the organization (although I have been trying to get the 'eve' name which it is taken already). Reason number one, I am doubtful we'll ever be able to gather all the different extensions/tools under the org umbrella. A possible alternative for EVE-SQLAlchemy would be to simply transfer ownership to the new maintainer.

leedhamj commented 7 years ago

Just to second @joystein's post.

We are in the process of doing some work with EVE/EVE-SQLAlchemy, based off the 0.6 port @dkellner did. I'm happy to contribute any code modifications we do, but whether we would be in a position to contribute the the management structures you are talking about I'm not so sure. Maybe. But I'm certainly keen to see an official 0.6+ version released.

What exactly is needed to bring that about do we think?

joystein commented 7 years ago

@nicolaiarocci : I'm not sure it necessarily should be a goal to gather all projects under the same umbrella, but i think it could hold value to offer GH org structure to those projects that are interested and could benefit from it, and then in particular with continuity in mind.

As for the name of a potential org, did you try python-eve as in the domain name?

In pure self-interest of seeing a updated version on pip (and partly as a response to @leedhamj), i guess i can try to list the options that see from my pow (in no way am i pretending to have any influence here, so please do not feel offended if i make unreasonable assumptions about your interests and availability):

A: Maybe the quickest to a release, but maybe not ideal in the long term:

  1. @amleczko grants access to @dkellner to the current repo so that the 0.6 port can be merged and see wider testing by those who have shown interest in this thread.
  2. @amleczko adds @dkellner and/or @nicolaiarocci as owner and/or maintainer of eve-sqlalchemy on pypi (see https://docs.python.org/3.6/distutils/packageindex.html#pypi-overview for more information about how this can be done) so that a relase can be done.

B: Might take longer, but might be better for long term:

  1. @nicolaiarocci creates an GH organization for eve.
  2. Access is granted to @amleczko so that eve-sqlalchemy can be migrated to the GH org.
  3. Access is granted also to @dkellner so that 0.6 branch can be merged and tested.
  4. @amleczko grants access on pypi to @nicolaiarocci and/or @dkellner so that a release can be published.

C: A combination is of course also possible, where A is done first, and then the project is transferred to a GH org at a later stage. Access to pypi is granted/transferred to GH org.

D: A proper fork i guess is worst case:

  1. @nicolaiarocci creates an GH organization for eve.
  2. The fork with 0.6 is setup on the GH eve org as a proper fork, maybe with a new name. @nicolaiarocci grants access to de facto maintainers as he sees fit.
  3. The forked project is uploaded to pypi under a new name where @nicolaiarocci holds the main owner credentials, and he grants access to de facto maintainers as he sees fit.

E: Of course someone else could also fork and evolve the current project to become the authoritative eve-sqlalchemy project over time.

The possibilities hinges a lot on @amleczko availability and willingness to grant access to others. For a first release, i believe one also would greatly benefit from assistance from @dkellner to get it out the door.

dkellner commented 7 years ago

@amleczko: Welcome back, nice to hear from you :). I really appreciate the work you've done so far, so no hard feelings because you did not respond for some time. We just have to find a proper solution on how to continue working on Eve-SQLAlchemy to provide a stable an up-to-date alternative to the default Mongo data layer.

@nicolaiarocci: In my opinion, we should go for an organization. Even if we could just gather Eve and Eve-SQLAlchemy there we would already benefit. But to be honest I did not create one yet so I don't know how much of a hassle that is. And as far as I remember you maybe planned to release Eve-Mongo as a separate package too - it would fit perfectly in that organization, too.

I can principally maintain further development of Eve-SQLAlchemy - I just don't know how much time I can spare the next weeks. It's going to be Christmas soon and that means peak season at my company ;).

nicolaiarocci commented 7 years ago

Thanks for the input folks. Really appreciated.

amleczko commented 7 years ago

I've just added @dkellner and @nicolaiarocci as collaborators of Eve-SQLAlchemy. Welcome guys.

nicolaiarocci commented 7 years ago

Thanks for adding me to the project. But everyone, keep in mind that I am not involved with the eve-sqlalchemy project itself.

@dkellner feel free to start working on the project, try and break things, etc. 😄

amleczko commented 7 years ago

Actually @nicolaiarocci you are also the repo admin so you can add new collaborators in the future.

dkellner commented 7 years ago

As I said, I won't have much time for this before christmas this year. But after that the first goal would be to have compatibility with Eve 0.6 (and 0.7) and then clean up some redundant code. Many of the tests have been copied over from Eve and I'm sure we'll all benefit from datalayer-agnostic integration tests wherever they make sense.

nicolaiarocci commented 7 years ago

@dkellner excellent!

leedhamj commented 7 years ago

That's good timing - we will probably have some bits to contribute after Christmas, especially around polymorphism and nested structures, won't we @coatespixit.

bomb-on commented 7 years ago

Hi all! May I ask what is the progress with this at the moment? Do you need more developers around it and if so how can one join the team? :)

dkellner commented 7 years ago

The next step is to review the changes I've made more than one year ago and merge the PR https://github.com/RedTurtle/eve-sqlalchemy/pull/92 . I think I can do that within the next week. After that I will try to get some overview of open issues and PRs, I didn't have much time around Christmas and up until now :). And then try to release a new version on PyPI, which might be most important for most people.

Developers are always welcome: Just try to address an open issue or (maybe even more important) update the docs and examples with more sophisticated use cases.

bomb-on commented 7 years ago

@dkellner Alright, so I tried to clone the repo and tried to run tests before reading #92 :) Yeah, you can imagine bunch of red-coloured text in my terminal... I tried to play around with the problem I have with JSON data (#124), but I'm not sure is there any point to go into this before testing (maybe even Eve 0.7 compatibility) is sorted out? I'm even ready to start working on #92, but I'm not sure how much progress you (or anybody else) did?

dkellner commented 7 years ago

@bomb-on How did you try to run the tests? Using tox they pass for both eve-0.6 branch in my fork and the master branch in this repo. If you cannot verify this please open a new issue so we can continue the discussion there.

running tox for branch eve-0.6:

...
___________________________________ summary ____________________________________
  py26: commands succeeded
  py27: commands succeeded
  py33: commands succeeded
  py34: commands succeeded
ERROR:   pep8: commands failed

running tox for branch master:

...
___________________________________ summary ____________________________________
  py27: commands succeeded
  py33: commands succeeded
  py34: commands succeeded
ERROR:   pep8: commands failed

I will fix the PEP8 issues before merging #92.

bomb-on commented 7 years ago

@dkellner Ah, yes, sorry, my bad :)

Running both (master branch from this repo and eve-0.6 from your fork) with tox resulted with success (except PEP8, of course).

I wonder how to proceed now? On which repo and branch should I work? Do we still work with Eve 0.6 or should we focus on 0.7 although it is still in development? What about coverage?

Thanks for clarification and all the info! ;)

dkellner commented 7 years ago

OK, I've merged #92 just now. The docs still need to be updated and some other PRs wait to be reviewed and merged before we can release 0.5. I've tagged some PRs with a milestone 0.5 already and will continue to do so as I regard them necessary for a good "first" release after a long time.

@bomb-on (and all others, of course) You can work on the current master branch from now on. One of the most important things before releasing 0.5 is to make sure that everything is consistent, especially the docs. Any proof-reading and fixing is highly appreciated. And providing more sophisticated examples of how to use this, too.

bomb-on commented 7 years ago

@dkellner Awesome! Let's get busy!! :)

nicolaiarocci commented 7 years ago

Hey folks, I created the @pyeve organization and moved all my eve-related repos over there. Since we talked about it in this thread a while ago, if you're alright with that, I would move this repo too. Thoughts?

dkellner commented 7 years ago

@nicolaiarocci I'm definitely alright with that. I think it's a good thing for the community around Eve to have this one "umbrella organisation" to cover projects close to the core ;).

Do you happen to know if i will be able to edit e.g. the repository's description and/or tags after it's moved there and you've added me to the organisation? That's one thing I cannot do right now even I have commit access.

nicolaiarocci commented 7 years ago

You should be able to do that.

nicolaiarocci commented 7 years ago

Actually, I did not realize first but you were not Admin of this repo. You are now so you can already edit repo description and/or tags, etc.