> * Could we use 'check_reserved' instead of 'check_reserve'? It sounds more > natural to me.
Why not. > > * Capitalize SQL in the error message strings It is capitalized. > > * dal.py is already much too long. Could the wordlists live somewhere else? No, they belong with their relative adapters. Object Oriented devel at its best. > > * Is 'print' the most appropriate way to show the warning message? How about > at least sending it to stderr, or see the next point: > > * I don't entirely get the rationale for the distinction between the warning > message and raising a syntax error, at least not based on the selected list. > Since this is intended for development, not production, why not just make it > a syntax error and > be done with it? Or if it's important, make it a > separate option. > here is my rational. the reason for displaying an error is because the KEYWORDS_ALL contains all known sql keywords. Therefore it contains keywords that might not necessarily effect your deployment, so it shouldn't stop the software from running, but at least log a warning letting you know so that you can change it if you want. Sure we can print to stderr. KEYWORDS_ALL is a placeholder until we devote the time to creating adapter specific lists. In fact once each adapter has their own list KEYWORDS_ALL probably won't exist any more, and the `all` option will use a union of all the lists. You said it yourself, you don't want more proliferating flags, so no separate option. However if anyone else has an opinion on the matter? > > * I remain uneasy about the idea of doing this on every call (vs calls that > make actual changes in the db). I don't know what the performance overhead is > of the lookup (I suspect that frozenset would be faster--at the very least > something immutable), so maybe it's a non-concern. I'm also uneasy about > proliferating flags that you have to remember to turn off for production; > it's too easy to forget about once it's working. I don't understand what you are so concerned about the performance overhead (like a fraction of a fraction of a fraction of a tiny fraction of a nanosecond that it takes?) Let me qoute you again "Since this is intended for development, not production..." ...I made my case, as long as check_reserved is set to None it won't do the check. (None is the default) And about the flags... you have to change many other options when moving into deployment (such as db connection string to the DAL... which ill be damned if its not in the same constructor as this flag). So let me get this straight, you want the option to target multiple database types that YOU want... but what..., web2py is supposed to play God and read your mind? How else would web2py determine what target backends you are looking for. Then you raise a good point, there is no sense in adding a flag just for one option, perhaps a better name such as ``target_adapters=[]`` instead of check_reserve. That way the list could be used for multiple purposes that might arise in the future. such as higher compatibility layers that check joins vs. pagination etc. -- You received this message because you are subscribed to the Google Groups "web2py-users" group. To post to this group, send email to web...@googlegroups.com. To unsubscribe from this group, send email to web2py+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/web2py?hl=en.
