-
Notifications
You must be signed in to change notification settings - Fork 143
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
decimal on GAE #302
Comments
I was unable to duplicate the problem in pydal, but on examination I think the problem is in
Line 572 is in an 'if' block that uses the private attribute '_dbname' to determine if gae specific code should be run, the test evidently fails, because the non-gae code is being run: 564: if self.dbset.db._dbname != 'gae':
...
572: records = self.dbset(table).select(*fields, **dd)
573: else:
... There appears to currently be 4 different google adapters: # discouraged, for backward compatibility
ADAPTERS['gae'] = GoogleDatastoreAdapter
# add gae adapters
ADAPTERS['google:datastore'] = GoogleDatastoreAdapter
ADAPTERS['google:datastore+ndb'] = GoogleDatastoreAdapter
ADAPTERS['google:sql'] = GoogleSQLAdapter So the test: 613: from pydal.adapters import GoogleDatastoreAdapter
...
617: if GoogleDatastoreAdapter is not None and isinstance(self.dbset.db._adapter, GoogleDatastoreAdapter):
... Though, that is still a somewhat clunky way to solve the problem. If applications are going to need to know that they are using a GAE adapter, then there should likely be better support for determining that fact from PYDAL. But should the applications need to know this? I don't know. What is the PYDAL philosophy for dealing with backend specific code in the application? |
@stephenrauch thank you for the accurate analysis. I think dealing with specific db engine code is unavoidable, since we support very different engines. Finally, I think this should be fixed on web2py's code only. @mdipierro do you agree? |
The basic problem is that framework code like Web2py shouldn't really have any code specific to the backend used since it is outside of Web2py control. But in certain instances, Web2py will need to alter its behavior based on backend characteristics. One example is the function So while I agree the Web2py code should change, it may still be necessary for the pydal code to provide support for the change. I looked at the above mentioned code in Maybe someone else could suggest one or more capability queries which could solve the problem in |
The title on this issue looks to be a point of confusion. The link at the beginning of this post, links to a post by David Manns. The post has nothing to do with decimal, but does have the stack trace that was analyzed above. David Manns has another post about the same time about decimal, and converting/migrating integers to same, that is here: https://groups.google.com/forum/?fromgroups=#!topic/web2py/_YHapb2KGPM I don't know if we can change the title of this issue, but if so, it should be considered. |
Stephen Rauch is correct, this issue is mis-titled. Its still open and I just confirmed still broken in web2py 2.14.5. For me this means I'm stuck with web2py 2.9.12 in production as I can't think of an acceptable workaround for setting up the appropriate validators in my form: def transearch():
|
Above is the error log from the current web2py version 2.14.5. Yes, the 'gae' test is bad. But the 'else' clause is clearly an earlier attempt to make IS_IN_DB work in the GAE case. I forced the code to take the 'else' path by changing 'gae' to 'google:datastore' which is the value observed. It still failed. So the fix is going to involve more than just the test for the gae environment. Note that IS_IN_DB(db, ...) works fine, it is only IS_IN_DB(set, ...) that breaks. So its possible that changing the 'set' implementation might avoid the need for GAE specific code. |
https://groups.google.com/forum/#!topic/web2py/EZ37Ipakiaw
The text was updated successfully, but these errors were encountered: