search

graphql-python / graphene-gae

GraphQL Support for Google AppEngine [DEPRECATED - Looking for maintainers]
http://docs.graphene-python.org/projects/gae/en/latest/
BSD 3-Clause "New" or "Revised" License
117 stars 14 forks source link

Fix for PolyModel type lookup #41

Closed Emsu closed 6 years ago

Emsu commented 6 years ago

Note: When using graphene Interfaces, previous implementation could display false positives by only checking if the root type matches with the given model. Now if you pass in an Interface type, the check will check that the object type's class hierarchy is a subset of the instance's class hierarchy

Emsu commented 6 years ago

As an example, if you had the following models:

Notification (ndb.PolyModel)
|-JobNotification
|-ApplicationNotification

where JobNotification and ApplicationNotification where subclasses of Notification, and you checked if an instance of ApplicationNotification(ndb.Model) was a valid model for a NdbObjectType of JobNotification, it would return true instead of false. (It Checks that Notification is in [Notification, JobNotification] instead of if [Notification, ApplicationNotification], is in [Notification, JobNotification]).

The language I'm using might be a little confusing so let me know if you have any questions.

ekampf commented 6 years ago

@Emsu thanks for the fix but some tests are failing... can you fix so I can merge?

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 91.608% when pulling 65549d82eb8d6e89dc4f14fc92df43ef8e07b8de on Emsu:master into 751ab0f6886c476ac478943a99835c7a24485ee2 on graphql-python:master.

Emsu commented 6 years ago

@ekampf I've fixed the tests

It looks like tests are breaking due to upstream change of messages being unicode as well as slight error messaging changes. Perhaps we should freeze the graphene version and graphql-python version?

One other suggestion would be cutting a new release? I'm still working on graphene < 2.0 due to the releases in PyPi being the pre-graphene-2.0 graphene-gae release. Of course, if you feel the 2.0 release isn't ready yet, then I think it's fine not to release it, but then maybe master should be a continuation of graphene-gae 0.1.7 and we can keep the 2.0 branch separate.

ekampf commented 6 years ago

@Emsu I have migrated away from GAE a while back so I kinda have a hard time keeping this lib up to date. Would you like to become a committer here so you can help with releasing new versions?

Emsu commented 6 years ago

Sure @ekampf I'm happy to help out. I'm not working full-time with GAE so might not be the ideal candidate but I can at least do maintenance tasks and some small library updates.

ekampf commented 6 years ago

@syrusakbary can you please give @Emsu access?