humanize.ordinal is very English-specific

2010-03-23 Thread Shai Berger
Hi,

I've been doing some translation work and ran into the humanize.ordinal 
template tag; I found it extremely English-specific. It works by choosing 
suffixes ("st", "nd", "rd", "th") to add to numeric representation (turning 
e.g. 22 to "22nd") according to the last digit or two.

In my language, Hebrew, the whole notion of adding a suffix to a number to 
turn it into an ordinal doesn't exist. An ordinal filter for Hebrew would use 
full words for 1-10 and leave the number as is for all others. But the current 
code (as of r12841; the last committed change to humanize.py is 12199) can't 
even get it right for French: It makes the translator choose between missing 
on 1 (making it '1eme' instead of '1ere'), and missing on all numbers ending 
with [2-9]1 (adding 'ere' where it should add 'eme').

Is there any bug or discussion of this that I missed?

Assuming not, should I file one? What can be done? The instinctive thought is 
that the current implementation should move to localflavor, and an ordinal() 
in humanize.py should try to invoke an appropriate implementation, but that's 
not usually what localflavors do...

Thanks in advance,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: humanize.ordinal is very English-specific

2010-03-23 Thread Shai Berger
Hi again,

On Tuesday 23 March 2010, s.kuzmenko wrote (I'm assuming you intended this to 
go to the group, rather than just me):

> That's a hard one. In some languages those suffixes will depend on the noun
> they refer to. To elaborate further your French example, the equivalent
> of "1st" would be either "première" (1ère) for feminine or "premier" (1er)
> for masculine.

Right. In fact, that is the case for Hebrew as well, and there are further 
issues involving the definite article.

> I don't think it is possible to have a nice generic solution
> for generating correct ordinals in some languages.

Depends on your definition of "nice"... It might require linguistic tagging of 
verbose names. This may be easier than it sounds (though the race towards a 
release may not be the best time to discuss it).

> For those languages
> where ordinal forms do not depend on the context it would indeed make more
> sense to keep this feature in localflavor.
> 

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: humanize.ordinal is very English-specific

2010-03-26 Thread Shai Berger
On Friday 26 March 2010, Russell Keith-Magee wrote:
> However, if the OP (or anyone else) can propose a simple, low maintenance
> solution that will allow ordinal to support more languages than it does
> currently, I'm happy to entertain it.
> 
Ok, I have a 2-part proposal:

1) As I've hinted before, make humanize.ordinal defer to a locale-specific 
version. I'm not sure localflavor is the right container (those are usually 
for explicit use and are actually more country-specific than language-
specific), but something like (pseudo-python)

in contrib.humanize.templatetags

def ordinal(value):
if settings.USE_I18N:
loc = get_current_locale()
if loc:
try:
from contrib.localflavor.$loc.humanize import 
ordinal
return ordinal(value)
except ImportError:
# no specific version, but locale exists
return value
except:
# exception thrown from imported ordinal
return ""
# no I18N or no locale: use current code
...

This will only allow languages were ordinals are not inflected in any way, but 
it will already allow e.g. Dutch (where there is a suffix system similar to 
English, with special suffixes for 1,2,3, but those suffixes do not apply for 
numbers over 20).

2) Add an optional argument to supply inflection information

def ordinal(value, inflection=None):

The idea is to supply the inflection information via an "abuse" of the gettext 
system: The aware template writer will provide a placeholder for inflection 
information, and the translators can then fill it in. For example, assume a 
template takes the user's number of visits as 'visnum', and the number of 
their next step in some process as 'step'; it may say, then,

{% blocktrans with visnum|ordinal:_("+time+") as vis 
and step|ordinal:_("+step+") as ste %}
This is your {{ vis }} time here. Let's take your {{ ste }} step.
{% endblocktrans %}

Now, the French translator can provide, in the .po file (and when I say 
"pardon my French" here, I mean it literally; my French is pretty basic, but I 
figured French will be more communicative than Hebrew)


msgid "This is your %(vis)s time here. Let's take your %(ste)s step." 
msgstr "C'est votre %(vis)s fois ici. Prenons votre %(ste)s pas." 

#. inflection information for words accompanying the translation for 'time'
# here it is translated to 'fois', which is feminine
msgid "+time+"
msgstr "f"

#. inflection information for words accompanying the translation for 'step'
# here it is translated to 'pas', which is masculine
msgid "+step+"
msgstr "m"


The French humanize.ordinal can then be written like so:


def ordinal(value, inflection):
if value==1:
if inflection=='m': 
return '1er'
elif inflection=='f': 
return '1re'
else:
raise TranslationError("unknown inflection", inflection)
else:
return str(value)+'eme')


And things will behave correctly.

Notes:

-- In the above .po fragment, I put "extracted comments" ("#." lines) 
explaining what the "+word+" messages are for. Making those appear in the file 
should be possible, through some modification of makemessages, assuming a 
convention such as the '+' signs for marking them, but I haven't really looked 
into this.

-- Of course, it would be up to the people writing each language's humanize 
module to define what inflection information they need and in what form. 
Cross-linguistic conventions would probably be nice, though not necessary. The 
information will be useful for things other than ordinals, though, so gender 
alone (as above) will probably not be sufficient, even for French. Proper 
definition of the inflection information format is the tough cookie in all of 
this, and it may well be tough for each language separately.

-- Some other details need to be worked out; for example, in Hebrew, the 
number (single/plural) should be part of the inflection information, and I 
haven't really considered the interactions there.

-- There are languages where the inflection information can not be fully 
derived from an accompanying word (e.g. languages with case, where a word's 
form can change according to syntactic role, like Russian and Arabic). I 
surmise this can be handled by separating references, but I really don't know 
enough, neither about these languages, nor about gettext. However, 80%-20%.

-- This helps where the ordered entity is known in advance, it can't solve the 
generic case where the template is "{{ index|ordinal }} {{ object }}". I think 
getting there will take us some more time still.

Have fun,
Shai.

-- 
You received this message because you are subscribed to

Suggestion: Limit activated languages to settings.LANGUAGES

2022-10-04 Thread Shai Berger
Hello Djangonauts,

This suggestion is following from discussions of the security issue
which was resolved in today's release. In essence, the issue is that
language codes are optionally used as prefixes in URLs, and for this
use they also become part of regular expressions used by the URL
resolution mechanisms. So, if an attacker manages to convince the
server to use a "language code" which encodes a pathologically complex
regex, the server can be DOS'd. The solution was to modify the
regex-inclusion part to regex-escape the language code -- ensuring
that language codes are never interpreted, in this role, as anything
other than a simple chain of single characters. This is the proper
security fix: Prevents the problem where it could manifest, with
minimal effects elsewhere. 

But looking forward, I think we should reconsider the fact that
django.utils.translation.activate() will just activate whatever
language code it is given. We do have a setting, LANGUAGES, which
defines a list of the languages (and codes) supported by our site. Why
should activate() accept anything that is not in this list?

Two points have been raised in support of the current behavior: The
existence of custom languages, and of fallback language codes (that is,
e.g where the user asks for zh-hk and gets, instead, zh-hant). But in
my opinion, they do not justify it:

- A custom language should be included in settings.LANGUAGES if it is
  to be supported; otherwise, e.g. makemessages will not even handle
  its translation file.

- When a language with a fallback is requested, the site should really
  activate the fallback language, not pretend to activate the requested
  one while actually using the fallback. As an example, if "en-us" is
  used as a fallback for "en-gb", and the URL has "en-gb" in it, then a
  British user would rightly be offended by all the American spelling
  they would see. The site should be honest enough to say "yes, you
  asked for en-gb, but we fell back to en-us; sorry, that's the best we
  have for you".

Note that there are all sorts of functions that check if a language
code is valid. For example, django.views.i18n.set_language() checks if
a translation for the languages exists in the project or its apps (but
not, AFAICT, the setting). d.u.translation.get_language_from_request()
and get_language_from_path() do check the LANGUAGES setting. It is
likely that including the check in activate() will do some double work.
And yet, we found ourselves introducing the security fix.

(I should note that this suggestion was also, independently, raised by
Benjamin who reported the vulnerability)

Opinions, suggestions, and explanations of the value I miss in allowing
activate() to take random language codes welcome.

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/20221004170511.4342ebc7.shai%40platonix.com.


Re: Model-level validation

2022-10-10 Thread Shai Berger
I see two separate concerns here:

1) Should Django present to users the option to do validate-on-save by
default? That is, should that option be visible -- in the form of a
documented setting or an optional argument to save()?

I tend to accept James'  (and others) views and reasoning against that.

2) Can a user activate validation-on-save-by-default without resorting
to monkeypatching? Should it be possible?

I think applying such validation should be possible -- because, in many
places, you see less-than-disciplined teams creating large projects
containing heaps of code that is not of the finest quality. And I think
we should help these projects improve incrementally -- that is,
introduce means to improve their situation; promoting notions like

data mutation should occur only in well-defined, controlled ways

without making them prerequisite.

This notion is a nice ideal, but installing it on a project
after-the-fact is hard; in many places it is not realistically
attainable -- at least in the boundaries of the team's resources,
delivery requirements, and a reasonable timeframe.

Note that for such general validation, a project-wide-base-model as
suggested e.g. by Uri is, in general, not sufficient, because it may
not apply to models from 3rd-party apps, or even from django.contrib
apps. Most models in such apps are not swappable.

But there is a way, I think, using a pre_save signal. One can write a
small app to install pre-save validation on all models in the project,
merely by including it in INSTALLED_APPS.

Basically, 

from django.db.models import signals
...
class WhateverConfig(AppConfig):
...

def ready()

def validate(sender, instance, raw, **kwargs):
if not raw:
instance.full_clean()

signals.pre_save.connect(validate, weak=False)

This, of course, is just a POC -- it doesn't include things like
allowing a model to opt out, for example. Or one might want to apply it
only where settings.DEBUG is set. Or only log warnings. Or a few other
variations that don't jump to my mind immediately.

But it is a way for those of us involved with large teams and projects
to add this feature, without affecting the experience of newcomers or
the layer-separation sensibilities of the framework.

HTH,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/20221010163935.4ba0fcb6.shai%40platonix.com.


Re: [Technical Board?] Project Ideas, and beginning GSoC 2023.

2022-11-26 Thread Shai Berger
Hi,

Adding to the above, I have two migration-related ideas.

The first is quite down-to-earth: Support for moving models between
apps. This is a long-standing problem, esp. in enterprise-y or just
long-running projects. I have expressed my dissatisfaction with the
current state of things a couple of years ago[1], and maybe it's time
to change that.

The second is a bit of a pie-in-the-sky: Allow auto-detection of custom
migration operations. The auto-detector has its change-detection mostly
hard-coded into it[2], and it doesn't seem easy or even possible to
allow third-party code to intervene. But I dream...

Note: These two are quite independent. Auto-detection of the fact that
a model has moved between apps is not required, supporting such moves
with flags to makemigrations or even a dedicated management-command
would be completely satisfactory as far as I'm concerned.

Thanks,
Shai.


[1] https://forum.djangoproject.com/t/why-do-we-need-apps/827/20
[2] 
https://github.com/django/django/blob/main/django/db/migrations/autodetector.py#L104

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/20221126180231.372b01a2.shai%40platonix.com.


Re: Backport for ticket 34063?

2023-01-01 Thread Shai Berger
Hi,

I think at this point it would help to move the discussion forward, if
we tried to step beyond the specific issue and phrase the revision in
the backporting policy. This will let us, I hope, have a more
principle-based discussion.

If I get it right -- please correct me, James -- it would be something
like this addition:

"In a new domain of functionality, which is considered major and
central, bugs which would have been release blockers if found in time
will be considered as candidates for backporting if found within the
next two LTS versions" -- or even "... if found before use of the new
domain of functionality becomes mainstream" -- or something similar.

I think looking at it from that angle will be more fruitful. I will say
that looking at this principle, thinking about the vicious cycle
mentioned by James, I tend towards accepting his arguments.

We may want to phrase it a different way: Think of such major domains
as "experimental". We did that in the Python3 transition -- we had
"experimental support" from 1.5, and IIRC that "experimental" label
wasn't dropped until 1.8. I doubt we can retroactively declare async
views as still experimental, but we can modify the backporting policy
to say "release-blocker-level bugs in experimental features will be
candidates for backporting as long as the feature is experimental";
and we can set an exception that says "async is still experimental for
backporting considerations", in view of the little use we've seen so
far.

(I can see the argument against the last proposition, that says
"experimental means potentially broken, so it should be less worthy of
backports rather than more"; I disagree, because (a) we do want to
encourage such experimentation, and (b) no, it doesn't really mean
potentially broken, it means the API is not yet covered by the
stability guarantees; we're at more liberty to change things when we
fix)

HTH,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/20230101172133.29c41f34.shai%40platonix.com.


Re: get_manager short ut function proposal

2023-01-01 Thread Shai Berger
On Sun, 1 Jan 2023 16:33:45 +0200
Ramez Ashraf  wrote:

> 
> Interested or do you think the function can be enhanced it to make
> more useable for your everyday other cases ?
> 

This is half-baked, just a thought, but maybe you can take it some
place interesting:

Imagine a class that "collects" calls for later execution. Something
like (this exactly won't work, details below):

from functools import partialmethod
from django.db.models import QuerySet as QSet

class QS:
def __init__(self):
self._calls = []
def filter(self, *args, **kw):
self._calls.append(
partialmethod(QSet.filter, *args, **kw)
) 
return self
def annotate(*args, **kw):
# ... (same idea)
# ... other relevant methods, 
def __call__(self, qset):
for call in self._calls:
qset = apply(call, qset)
return qset

This won't work, I think, because partialmethod isn't supposed to work
quite this way, and the "apply" in the last line isn't defined. But
with this (fixed) already you could, I think, do something like

def qset_manager(qs: QS) -> Manager
class CustomManager(models.Manager):
def get_queryset(self):
orig = super().get_queryset()
return qs(orig)
return CustomManager()

And then, in your model, 

class Person(models.Model):
...
authors = qset_manager(QS().filter(role="A"))

and now the "make a manager with a modified queryset" pattern is
shortened in a general way.

As I said, just a half-baked thought. There's problems here to solve,
and many improvements to make.

HTH,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/20230101190053.671e8303.shai%40platonix.com.


Re: Request for Guidance

2023-01-18 Thread Shai Berger
Hello,

On Wed, 18 Jan 2023 21:55:11 +0530
"'12_Gairick Dam' via Django developers  (Contributions to Django
itself)"  wrote:

> Hello please suggest me some easy good first issues to contribute
> 

The Django issue tracker has tickets marked as "Easy pickings".

You can see the search for them here:
https://code.djangoproject.com/query?status=assigned&status=new&easy=1&desc=1&order=id

For further guidance and advice, I think you may find the Django Forum
(at https://forum.djangoproject.com/ ) more helpful.

Welcome aboard, and have fun,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/20230118211647.683ffd04.shai%40platonix.com.


Re: Feature request: making gettext more robust

2023-06-17 Thread Shai Berger
Hi Gergely,

On Fri, 16 Jun 2023 16:46:31 +0200
Gergely Kalmár  wrote:

> 
> I'm still thinking that it should be possible for Django to wrap
> gettext in a way that allows us to raise exceptions. It seems silly
> to me that we could not control this core aspect of the process.
> 

I think indeed it is possible. Take a look at the code in
django/utils/translation/trans_real.py and in particular, the
TranslationCatalog class; I _think_ you should be able to insert a
"fallback" catalog which raises some non-KeyError exception in its
`__getitem__()`, and that should give you what you want.

Note that it may be a little complex -- the mechanism there is built to
handle not only fallback languages (i.e. "en-US => en"), but a set of
catalogs for each language (i.e. collecting the translations into one
language from different apps), and further, the translations in each
language need to be kept separate because each may have its own
pluralization formula (in English, last I checked, there is only
one rule -- if the number is not 1, it's plural -- but if you specify
this rule in the .po file, the formula you get is technically distinct
from the default. Other languages sometimes have actually different
rules in different files, mostly for historical reasons).

I'm explicitly not expressing an opinion about whether this is
desirable :).

HTH,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/20230617170050.562f0d01.shai%40platonix.com.


Can we drop Oracle 9 support?

2011-11-16 Thread Shai Berger
Hi,

Oracle 10 has been released in 2003 -- the same year as Python 2.3, which is 
no longer supported by Django.

Oracle 9 is essentially end-of-life'd -- Oracle will not provide patches for 
it, even for critical bugs, and even if you pay extra. This is since July
2010, according to 
http://www.oracle.com/us/support/library/lifetime-support-technology-069183.pdf

Support for Oracle 9 is a very minor thing in the backend code itself -- only 
a handful of lines -- but these lines execute a command on the backend to get 
the server version, at every connection, and our data indicates this command 
does affect performance. You might expect that it just looks up a constant,
but in fact it is a stored procedure that looks in several places to calculate
not just the software version, but also the interface compliance level.

In the backend, the version info is only used to tell if we're still on 9. If 
we drop 9 support, we don't need that anymore.

For the benefit of those who need to know the version (e.g. 3rd-party code 
which uses specific Oracle 11 features), I propose we change oracle_version 
from an attribute on the connection that is filled at connection time, to a 
property that proxies the cx-oracle connection property. That property is 
memoized (at the connection-object level) anyway, so for code that relies on
it, the only difference would be in when the server statements are executed,
while for the rest (most?) of us, the statements will just not be run.

So -- can we drop Oracle 9? Should I open a ticket for it?

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Can we drop Oracle 9 support?

2011-11-23 Thread Shai Berger
On Tuesday 22 November 2011, Anssi Kääriäinen wrote:
> 
> I can do a patch for this if needed. Are you (or somebody else)
> working on this?
> 

While you're at it, remove the python-2.3-isms from the cursor's fetchmany and 
fetchall (base.py lines 722-730 in current trunk):


   return tuple([_rowfactory(r, self.cursor)
  for r in self.cursor.fetchmany(size)])

just remove the square brackets, there's no longer need to create a list as 
well as a tuple.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Suggested Oracle double-fetch fix (was Re: Can we drop Oracle 9 support?)

2011-11-24 Thread Shai Berger
On Thursday 24 November 2011, I wrote:
> On Tuesday 22 November 2011, Anssi Kääriäinen wrote:
> > I can do a patch for this if needed. Are you (or somebody else)
> > working on this?
> 
> While you're at it, [...]

While we're all at it, I would like to bring to your attention another 
suggested fix for Oracle. The patch attached here tells the backend that if a 
fetch operation retrieved less than the requested array size, then no further 
fetch attempts are necessary.

The fix is motivated by our DBA's finding, that many single-row-selects are 
followed by double fetches. When I checked the code, I realized that with the 
exception of aggregates, this is true for all single-row selects: whether it 
originates from qset.get(...) or from qset[j], the QuerySet code adjusts the 
SQL and then calls fetchmany in a loop until no results are returned. Since 
neither cx_oracle nor the Oracle client library take it upon themselves to 
stop such requests, they go all the way to the database.

The Python DB-API[0] says on fetchmany:
 """
The method should try to fetch as many rows as indicated by the size
parameter. If this is not possible due to the specified number of rows not
being available, fewer rows may be returned.
"""
I interpret that as "no rows should be fetched after a call failed to fill the 
buffer". Under a loose-enough transaction isolation level, rows may "come into 
being" in mid-transaction, but I don't think it is reasonable to expect them 
to do so in mid-statement.

That being said, the problem that this patch fixes is a little hard to test 
for; it essentially requires mocking the database library. Also, I'm not sure 
this is the right fix; I don't know if other backends have the same issue. If 
they do, it might be better to make qset[j] just use fetchone instead of 
fetchmany, like the aggregates do; and it might be worthwhile to also change 
the qset.get() logic (in a more subtle way -- we don't want to lose the check 
for multiple records returned).

As for the patch, it is only "tested" in the sense that one Django-1.2 based 
application continues to run correctly with (essentially) it; I don't(yet) 
have an Oracle+Trunk testing environment.

Looking forward to your comments,

Shai.


[0] http://www.python.org/dev/peps/pep-0249/

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Index: django/db/backends/oracle/base.py
===
--- django/db/backends/oracle/base.py	(revision 17145)
+++ django/db/backends/oracle/base.py	(working copy)
@@ -643,6 +643,7 @@
 charset = 'utf-8'
 
 def __init__(self, connection):
+self._all_fetched = False
 self.cursor = connection.cursor()
 # Necessary to retrieve decimal values without rounding error.
 self.cursor.numbersAsStrings = True
@@ -664,6 +665,7 @@
 return [p.smart_str for p in params]
 
 def execute(self, query, params=None):
+self._all_fetched = False
 if params is None:
 params = []
 else:
@@ -688,6 +690,7 @@
 raise utils.DatabaseError, utils.DatabaseError(*tuple(e)), sys.exc_info()[2]
 
 def executemany(self, query, params=None):
+self._all_fetched = False
 try:
 args = [(':arg%d' % i) for i in range(len(params[0]))]
 except (IndexError, TypeError):
@@ -720,10 +723,15 @@
 return _rowfactory(row, self.cursor)
 
 def fetchmany(self, size=None):
+if self._all_fetched:
+return DatabaseFeatures.empty_fetchmany_value
 if size is None:
 size = self.arraysize
-return tuple([_rowfactory(r, self.cursor)
-  for r in self.cursor.fetchmany(size)])
+rows = tuple(_rowfactory(r, self.cursor)
+ for r in self.cursor.fetchmany(size))
+if len(rows) < size:
+self._all_fetched = True
+return rows
 
 def fetchall(self):
 return tuple([_rowfactory(r, self.cursor)


Caching back-refernces on one-to-one fields

2012-01-18 Thread Shai Berger
Hi Django devs,

I have a small improvement to suggest for one-to-one fields: Make them cache 
back-references on related objects. That is, assume

class A(model):
pass

class B(model):
a = OneToOneField(A)

a1 = A.objects.get(...) # assume a1 has a related B record
b1 = B.objects.get(...)

Today, both (a1.b.a is a1) and (b1.a.b is b1) are false; each of the 
expressions generates two database hits. I propose to fix this.

I've run into the issue while doing some performance-improvement work on a 
large project; in this project, the painful instance of the problem was a 
user-profile model. The developers used the reverse of the 1-to-1 on the 
profile 
model instead of django.contrib.auth.models.User.get_profile(), and they often 
navigated between the user object and the profile object in both ways 
(sometimes even for good reasons), generating redundant database hits.

I wrote tests for the problem, and a fix, in the context of the project; I've 
now ported it to a fix in Django itself. It is a small fix, and I think beyond 
its general value, it will allow removing the special-case-caching from 
User.get_profile().

Do you see a reason why I should not post a ticket?

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: First-run experience w/ psycopg2 and django 1.3.x

2012-01-18 Thread Shai Berger
On Thursday 19 January 2012, Jeremy Dunck wrote:
> There's a known problem w/ latest pg (2.4.2) and we decided not to
> backport the fix to 1.3.x because there's a workaround.
> 
> https://code.djangoproject.com/ticket/16250
> 
> All well and good, but someone coming to Django w/ postgres will have
> a bad first-run experience because the docs don't mention this issue.
> 
> I think this should be mentioned here:
> https://docs.djangoproject.com/en/1.3/topics/install/#database-installation
> https://docs.djangoproject.com/en/1.3/ref/settings/#std:setting-DATABASE-EN
> GINE https://docs.djangoproject.com/en/1.3/intro/tutorial01/
> 
> Where else might it be useful?

I think first and foremost (maybe even only)

https://docs.djangoproject.com/en/1.3/ref/databases/

Also, the issue should be revisited since psycopg 2.4.4 was released lately.

Shai,

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Caching back-refernces on one-to-one fields

2012-01-19 Thread Shai Berger
Hi again,

> 
> john = Author.objects.get(name='john')
> books = list(john.book_set.all())
> 
> # Does a database query, but should be smart enough to simply return john.
> books[0].author
> """
> 
> I'm pretty sure there's a long-standing ticket for this, but I'm not
> sure which one it is. Shai, does your solution approach this in a way
> that can solve the issue for ForeignKeys as well?
> 
Only the tests...

My solution is a little modification on the descriptors used for the related 
objects. But for foreign keys, the relevant descriptor returns a manager which 
uses the QuerySet of the related model's default manager. To get the desired 
behavior, that manager should modify (or wrap) this QuerySet, to seed the 
cache on each object it returns.

I may have time to work on something like this, but probably only next week.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



django.contrib.auth.models.get_hexdigest missing in 1.4

2012-05-13 Thread Shai Berger
Hi,

In the process of upgrading to Django 1.4, I've run into a little problem: one 
of our files uses django.contrib.auth.models.get_hexdigest, which is no longer 
present.

I searched, and could find no mention of this in release notes, tickets, or the 
developers group.

Was this, for some reason, not considered a public API?

Of course, I can copy the function from a 1.3 installation and use that, but I 
suspect that if it was removed, there was a good reason -- and I'd like to 
know what it was, before deciding to ignore it...

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: auth_permission column lengths

2012-06-20 Thread Shai Berger
Hi all,

On Tuesday, June 19, 2012 5:12:47 PM UTC+3, Florian Apolloner wrote:
>
> Hi Greg,
>
> Django itself can't change that currently since there is no support for 
> schema alteration in Core. Once we get that we can tackle issues like that 
> and increase to a sensible limit. (both name and codename might cause 
> problems…).
>
> Cheers,
> Florian
>

I have suggested, on the bug itself, a partial solution that requires no 
database schema change (elide the "name" field if it's too long,
as it is only used for display anyway).

The "codename" field will then stay problematic, but the limitation there 
is much less severe.

Have fun,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/ly9RVtN53MsJ.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Django Oracle backend vs. numbers

2012-08-29 Thread Shai Berger
Hi Django developers,

I've been working with Oracle on a project with some performance requirements, 
and we've noticed long ago that the Django Oracle backend performs 
significantly worse than other backends; we've been able to profile it and see 
that, indeed, a lot of time is spent in processing the returned rows, while 
the Oracle DBMS itself is not breaking a sweat.

This week, I found what is, I think, the main culprit: The Oracle backend 
prefers accuracy to performance. Since Oracle numbers can have more significant 
digits than C doubles (=CPython floats), and the backend wishes to preserve 
this accuracy, it makes cxOracle (the Python Oracle client library) return all 
numbers as strings, and then converts the strings back to numbers -- ints, 
floats or decimals -- according to the Oracle data type descriptions.

To make things even slower, the conversion decisions are made anew for each 
row; this is done in the function _rowfactory (currently at 
https://github.com/django/django/blob/master/django/db/backends/oracle/base.py#L790).
 
The function itself is called less efficiently than possible (on rows returned 
from cxOracle, instead of within cxOracle before the rows are returned).

For our application -- as for many others, I guess -- precision of 38 digits, 
on real numbers or integers, is not important. I made a modified backend which 
returns numbers as numbers and the improvement in performance is very 
significant. A row-factory function is still required to make sure strings are 
returned as unicode, and creating such a function per query to only go over 
the string columns may improve things even more, but I didn't even have to go 
there yet.

Can we put this into Django core? Perhaps with a Settings knob for people who 
really need all the precision?

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-08-30 Thread Shai Berger
Hi Ian, Jacob, Anssi and all,

Thanks for the quick and positive responses.

On Wednesday 29 August 2012, Ian Kelly wrote:
>https://github.com/ikelly/django/commit/086f502a1c2acb9db27531f7df78cb9df9ab83bd
> 
> Shai, if you would be willing to try this patch, I'd like to hear your
> results.  Please bear in mind that this is currently only a POC, so it
> could be buggy -- I've only run it through the basic tests.
> 

This looks very promising, I will try it next week. On a first look, there is 
just one thing about it that I don't like, and it is related to Jacob's 
comments --

On Wed, Aug 29, 2012 at 10:34 AM, Jacob Kaplan-Moss  
wrote:
> I'm generally adverse to introducing more settings -- settings creep
> is a real problem -- so a couple of alternate ideas you might consider
> in producing a patch:
> 
> 1. Just pick one correct behavior, probably for each type. My guess
> would be that ints and floats should get the fast behavior, and
> decimals the slow-but-correct one. If you're using a decimal field
> it's because you care about precision, so correctness makes more sense
> there IMO.
> 
This would definitely be preferrable to a setting, but it doesn't cover all 
cases -- we have some raw queries where expressions are being selected. The 
default number type in this case -- in current code as well as Ian's patch -- 
is to return the data as a string and convert it to int or Decimal. Our code, 
which was written against other database systems, tries to use the result in 
arithmetic with floats, and this fails (you can't add Decimals to floats).

I could, of course, prevent the exception by converting the value to float in 
my code, but that feels like the wrong solution -- I know, in my case, that 
float is enough, and I'd much rather tell the backend "give me floats" than 
have 
it go through float->str->Decimal->float.

> 2. If you must introduce a setting, do it in DATABASES[OPTIONS]
> instead of a top-level ORACLE_FAST_NUMBERS or whatever.

That is indeed one good option. The alternative I'm considering -- less 
convenient for me, but perhaps more flexible generally -- is, since we're 
dealing with raw SQL anyway, to put some API for this on the cursor; perhaps a 
property, so that setting it would be a no-op on other backends.

As per Anssi's comment, I have no backwards-compatibility requirements with 
respect to cx_Oracle, but we are still running Django 1.3. Trying to call the 
performance improvement a "bugfix" so it can be backported, while changing 
cx_Oracle requirements, would be pushing my luck, right?

Thanks again,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-03 Thread Shai Berger
On Wednesday 29 August 2012 22:17:22 Ian Kelly wrote:
> 
> https://github.com/ikelly/django/commit/086f502a1c2acb9db27531f7df78cb9df9a
> b83bd
> 
I've run a version based on this -- basically, porting the patch to Django 1.3
which we use (so, no tzinfo), using "float" as the default for expressions, and
putting the whole thing as a derived class from the original adapter rather
than a patch (because that's what we do here, as we also need named parameters
and we had to add them). I found that it leaks something -- the error says
"cursors", but I suspect something more subtle; either way, this is what I got
from a page which makes several hundred queries (don't ask):


Traceback (most recent call last):

  File ".../lib/python2.7/site-packages/django/core/servers/basehttp.py", line 
283, in run
self.result = application(self.environ, self.start_response)

  File ".../lib/python2.7/site-packages/staticfiles/handlers.py", line 66, in 
__call__
return self.application(environ, start_response)

  File ".../lib/python2.7/site-packages/django/core/handlers/wsgi.py", line 
274, in __call__
signals.request_finished.send(sender=self.__class__)

  File ".../lib/python2.7/site-packages/django/dispatch/dispatcher.py", line 
172, in send
response = receiver(signal=self, sender=sender, **named)

  File ".../lib/python2.7/site-packages/django/db/__init__.py", line 85, in 
close_connection
conn.close()

  File ".../lib/python2.7/site-packages/django/db/backends/__init__.py", line 
244, in close
self.connection.close()

DatabaseError: ORA-02002: error while writing to audit trail
ORA-00604: error occurred at recursive SQL level 1
ORA-01000: maximum open cursors exceeded

Thanks,
Shai

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-04 Thread Shai Berger
On Tuesday 04 September 2012, Ian Kelly wrote:
> On Mon, Sep 3, 2012 at 6:14 AM, Shai Berger  wrote:
> > On Wednesday 29 August 2012 22:17:22 Ian Kelly wrote:
> >> https://github.com/ikelly/django/commit/086f502a1c2acb9db27531f7df78cb9d
> >> f9a b83bd
> > 
> > more subtle; either way, this is what I got from a page which makes
> > several hundred queries (don't ask):
>
> Are these ORM queries or custom queries?

Mostly ORM.

> If the latter, are you
> explicitly closing the cursors?  That should help prevent the issue,
> although unfortunately the ORM doesn't do this.  The cursors should be
> closed anyway when they're finalized, but maybe we've got a reference
> cycle preventing them from getting cleaned up right away, and too many
> are accumulating between garbage collection passes.
> 
That would make sense. 

> When I've seen this error in the past, the immediate solution has been
> to increase the OPEN_CURSORS parameter in the database.  See:
> 
> http://www.orafaq.com/wiki/ORA-01000
> 
I'll try, but this shouldn't be the long-term solution -- especially not when 
an alternative exists, which does not have this problem.

BTW, I'm not at all sure that using outputtypehandler (function called per 
returned value) is more efficient than using rowfactory (function called per 
row) when both are used via the cx_oracle API. The current code calls its 
_rowfactory outside this API, which does seem inferior. If the rowfactory 
approach behaves better in terms of GC, I don't see why we should insist on 
outputtypehandler.

I'll try to prepare a patch for consideration, hopefully this week.

Thanks,
Shai.


-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-22 Thread Shai Berger
On Wednesday 05 September 2012, Ian Kelly wrote:
> 
> Thanks for tracking that down.  _rowfactory was a module-level
> function rather than a method, and I should have left it that way.

So -- Ian, Anssi, are you going to include the fix in Django 1.5? I can help by 
either reviewing it for you or providing the patch for you to review. 

If you let me write the patch, I will also include the extra setting in the 
"options" dictionary (choosing if expressions should be treated as decimals or 
floats). I think this is then an added feature, and needs to get in before the 
feature freeze. Otherwise, just fixing the performance can be treated as a bug 
in be included later. Is this guess correct?

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-24 Thread Shai Berger
On Sunday 23 September 2012 15:05:21 Anssi Kääriäinen wrote:

> 
> If you decide to work on this, please split the patch into two: one
> for the ._rowfactory change, one for the feature addition. My belief
> is that the ._rowfactory change is going to be something we can very
> easily justify committing into 1.5, but the proposed feature addition
> sounds like something which isn't as obvious.

While preparing the patch, I ran into the inspectdb regression test suite; 
this suite includes, among other things, a model with a database column named 
'prc(%) x', which breaks test database creation on Oracle on current trunk. I 
intend to include a fix for this (in a separate commit, of course) in the first 
part as well (doubling percent signs in quote_name()).

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-25 Thread Shai Berger
On Sunday 23 September 2012, Anssi Kääriäinen wrote:
> Doing final polish for Ian's patch and providing benchmark results for it
> will get this patch closer to commit.
> 
I had hoped the Django test suite alone will provide a convincing benchmark, 
but apparently that is not the case -- on my laptop, with Oracle in a virtual 
machine, the basic suite takes 63 minutes with or without the patch. This 
makes sense, because the effect is likely to be evident mostly in queries 
returning many rows, and those are not very common in the test suite.

I do not have a working Django-trunk project to test on, and this evening and 
tomorrow is a very sacred holiday here (Yom Kippur), so it might take me until 
the end of the week to produce benchmark results. If anyone wants to chime in, 
the pull request is https://github.com/django/django/pull/393. There is, so 
far, anecdotal evidence of very significant improvements, from Ian Kelly 
(original author of this patch) and myself (on a somewhat different 
implementation and older Django versions, but same general idea). In my case, 
page load times were cut by dozens of percents.

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-25 Thread Shai Berger
On Tuesday 25 September 2012 10:03:11 Anssi Kääriäinen wrote:
> 
> Am I correct the benchmark is basically:
> 
> cursor = connection.cursor()
> cursor.execute("some sql fetching many rows with NUMBER() columns")
> timer.start()
> for row in cursor.fetchall():
> pass
> print timer.used()
> 
Yes, I think so. I find timeit, from the python stdlib, very helpful for 
benchmarks.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-26 Thread Shai Berger
On Wednesday 26 September 2012 20:26:11 Andre Terra wrote:
> On Mon, Sep 24, 2012 at 9:41 PM, Shai Berger  wrote:
> > While preparing the patch, I ran into the inspectdb regression test
> > suite; this suite includes, among other things, a model with a database
> > column named
> > 'prc(%) x', which breaks test database creation on Oracle on current
> > trunk. I
> > intend to include a fix for this (in a separate commit, of course) in the
> > first
> > part as well (doubling percent signs in quote_name()).
> 
> For the record, I believe the relevant ticket and discussion are located
> at:
> 
> https://code.djangoproject.com/ticket/18843
> 

No and maybe: I think it's the "other side" of the same issue. The ticket is 
about problems in inspection; the problem I ran into is in creating a table, 
where the column name is given in the model. My code does nothing to fix the 
inspetion (and indeed, there are failures in the test suite with it); but 
without some fix to the problem I solved, the test suite cannot be run at all.

It may be the problem Anssi refers to in his comment there, I'm not sure.

I can open a ticket for this if core devs like, but it feels like unnecessary 
procedure when it is included with a fix that has already been discussed.

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-26 Thread Shai Berger
On Tuesday 25 September 2012, Ian Kelly wrote:
> On Tue, Sep 25, 2012 at 11:47 AM, Ian Kelly  wrote:
> > Thanks for putting together a pull request.  Please note the patch
> > will also need to update the documentation to state the new cx_Oracle
> > version requirement (was 4.3.1, now 5.0.1).
> 
> Also, there should probably be something about this in the 1.5 release
> notes.

Yep, forgot about documentation, thanks. PR updated.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-27 Thread Shai Berger
Hi all,

On Sunday 23 September 2012 15:05:21 Anssi Kääriäinen wrote:
> 
> Doing final polish for Ian's patch and providing benchmark results for it
> will get this patch closer to commit.
> 

I made some initial benchmarks, and they do not reflect the change I've seen in 
the actual application. I don't yet know what to make of this. With this 
model:

class AllSorts(models.Model):

start = models.IntegerField()
square = models.IntegerField()
sqrt = models.FloatField()
cubicroot = models.DecimalField(decimal_places=20, max_digits=25)
name = models.CharField(max_length=100)
slug = models.SlugField()

Calling this function for fetching:

@transaction.commit_on_success
def getAllSorts():
errors = 0
print "Getting %d objects" % AllSorts.objects.all().count()
for alls in AllSorts.objects.all():
if alls.cubicroot.is_nan():
errors+=1
print "Got %d errors in the decimal" % errors


I made 10,000 records. Getting them all (and making sure the decimal did not 
return a NaN) takes ~1.95 seconds on the old code, ~1.75 seconds on the new 
code, and ~1.6 seconds on the new-new code (where we use a new float_is_enough 
option, which tells the backend to only use Decimals for DecimalFields).

Adding a calculated expression:

@transaction.commit_on_success
def getAllSorts():
errors = 0
print "Getting %d objects" % AllSorts.objects.all().count()
for alls in \
  AllSorts.objects.all().extra(select={'e':'"START"+(2*"CUBICROOT")'}):
if alls.cubicroot.is_nan():
errors+=1
print "Got %d errors in the decimal" % errors

Did not change the results significantly, despite my expectations.

I'm not sure yet what we did in the application that made this make a huge 
difference. I see a significant difference here, but not amazing (though ~20% 
is 
nothing to scoff at). Perhaps making more request of fewer rows, counter-
intuitively makes more of a difference. Perhaps more strings. But I know I saw 
a much bigger difference than this.

The new-new code (sans documentation, at this point) is available for your 
review and benchmarking as Pull Request 402, or the oracle-float-exprs branch 
in my fork. The benchmarking project is attached to this message.

Thanks for your attention,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



bench.tgz
Description: application/compressed-tar


Re: Django Oracle backend vs. numbers

2012-09-29 Thread Shai Berger
Hi, Sorry about the delay in writing this -- it was in my mind, but I forgot 
to let it out...

On Sunday 23 September 2012, Anssi Kääriäinen wrote:
> On 22 syys, 23:34, Shai Berger  wrote:
> > 
> > [...] I will also include the extra setting in
> > the "options" dictionary (choosing if expressions should be treated as
> > decimals or floats). I think this is then an added feature, and needs to
> > get in before the feature freeze. Otherwise, just fixing the performance
> > can be treated as a bug in be included later. Is this guess correct?
> 
> There isn't much time to do feature additions. It is possible to get
> the feature into 1.5, but you should not expect it to happen. As for
> the feature itself, I am not yet sure if it is a good idea or not. The
> basic problem is granularity: wouldn't you want to do this per query,
> or even per expression basis?
> 
No, I don't; the issue is cross-database compatibility. We have raw SQL 
statements calculating some expressions, and then use the returned values in 
calculations in Python -- adding and muliplying them with floats. This worked 
fine with MySql and the 3rd-party SQL Server backends, but blew up on Oracle 
because Decimals and floats don't mix very well:

>>> from decimal import Decimal as D
>>> D(1) + 1.0
Traceback (most recent call last):
  File "", line 1, in 
TypeError: unsupported operand type(s) for +: 'Decimal' and 'float'
>>> 1.0 + D(1)
Traceback (most recent call last):
  File "", line 1, in 
TypeError: unsupported operand type(s) for +: 'float' and 'Decimal'

I could, of course, solve it on a query-by-query basis, adding a cast to float 
that is a nop on other backends. That seems a little backward to me, and an 
unfair requirements of pluggable apps.

Similarly, any per-query or per-expression API would be added only when using 
Oracle, and at best be a NO-OP on other engines -- which means it would only 
be added by Oracle users, with the same consequences.

It seems much better to me to make it possible to run Oracle in a mode that 
makes expressions in raw sql have semantics that is compatible with other 
backends.

(would filing a bug with the above arguments make it more likely to include the 
feature in 1.5?)

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-29 Thread Shai Berger
Hi, and thanks for your attention. I know you must be extremely busy with 
other issues of the feature freeze. I just want to make some things clear:

> Based on the above I am -1 on having a flag that interprets NUMBER
> fields as floats. There is already an Oracle data type to use for
> this.
> 

The flag does not affect the interpretation of NUMBER columns with scale and 
precision. True, in its current form, it turns bare NUMBER columns (decimal-
precision floating point) to floats. But these are columns that are not 
generated by Django's built-in fields; if you have them, you are doing 
something Oracle-specific. The flag is mostly intended to improve 
cross-database 
compatibility; other than that, it offers a very clear trade-off and it is off 
by 
default. It really shouldn't hit anybody by surprise.

> > > I am not yet sure if it is a good idea or not. The
> > > basic problem is granularity: wouldn't you want to do this per query,
> > > or even per expression basis?
> > 
> > No, I don't; the issue is cross-database compatibility. 
> > 
> The problem seems to be that on Oracle, 1.1 is NUMERIC, and that is
> something which _is_ a decimal in Python. We should not change that.
> 
> To me it seems the answer is to use to_binary_float/to_binary_double
> in raw SQL queries. For example:
> 

But this is an Oracle-specific solution. Exactly the thing I'm trying to avoid.

> 
> I tested the pull request 393 (the speedup to ._rowfactory). Above
> test case, and the decimal case runs in around one second, the float
> case in 0.1 seconds. So, there is a regressions for decimals. On the
> other hand floats are way faster. Will investigate...
> 
I confirm your findings: Running with different parameters (10,000 rows) I 
found 
Decimals are ~1.8x slower, floats are ~3 times faster, and significantly, ints 
are ~2 times faster. I suspect in the typical Django application, most numbers 
are ints.

I investigated a little myself, and found the reason to be the redundant 
lookup in _decimal_or_int(): It was referring to Decimal as "decimal.Decimal". 
I moved that to a default argument, and got performance to be ~40% better than 
trunk for decimals too. The pull request is already updated.

> As for new features for 1.5: there is roughly a day before feature
> freeze. Getting any new ticket into 1.5 is really unlikely at this
> point, existing pull requests have priority.
> 
FWIW, the pull request exists (402).

Thanks for your help,

Shai.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-29 Thread Shai Berger
On Sunday 30 September 2012, Anssi Kääriäinen wrote:
> 
> I walked into this one... But, sorry, I will not have time for this
> pull request for 1.5.
> 
Ok, I can't say you didn't warn me this would be the case. Thanks a lot for 
the time you did spend on it,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django Oracle backend vs. numbers

2012-09-29 Thread Shai Berger
On Sunday 30 September 2012, Anssi Kääriäinen wrote:
> 
> cur.execute("select case when dbms_random.random > 0.5 then 0.1
> else 0 end from testtable")
> 
> Guess what is the type of the column? It is both Decimal and int,
> alternating based on the random result.
> 
> This seems like surprising behaviour to me. The problem is in
> cx_Oracle, not Django code. In raw cx_Oracle queries you get ints and
> floats mixed.

I tried this query in sqlplus, and the response looks like mixed ints and 
floats there too. I don't know a way to get a value's datatype inside Oracle, 
so it's hard to be sure, but it could be that cx_Oracle is only reflecting 
Oracle with this.

> I don't believe it is OK to return different types for
> the same column in one resultset. I will report to cx_Oracle mailing
> list and see if they have anything to say about this.

I'm curious as well.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Update on localflavor move

2012-10-13 Thread Shai Berger
On Saturday 13 October 2012, Tomas Ehrlich wrote:
> Salut Claude,
> 
> > I can work on this, however I'm wondering if it makes sense to re-add all
> > translations in all packages. Is adding Korean translation to Switzerland
> > localflavor useful?
> 
> I think it is. If someone translated Switzerland localflavor into
> Korean language it means that he might need it (for whatever reason).
> 

As long as the localflavor was in one .po file with the rest of Django, the 
reason may well have been "to keep the translation percentages in my language 
up".

I suggest to keep the original Django .po's available and easily accessible 
for the localflavor packages. in case someone really needs these translations, 
but I don't think it makes sense to require all the porting work upfront.

(Spoken as one who once took part in one of the translations)

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Django produce sql with table name different then specified in meta db_table

2012-10-31 Thread Shai Berger
Hi Michal and Django devs,

While for the most part, Jacob is correct in marking this as a usage question, 
there does appear to be something buggy here.

Note how Michal is abusing the db_table setting to select a name with a 
schema. He's looking for "protein_therapeutics" (<30) in "mnowotka":

> class ProteinTherapeutics(models.Model):
> #...
> class Meta:
> db_table = 'mnowotka\".\"protein_therapeutics'

This, almost worthy of being called an sql injection, can't be the right way 
to achieve the goal. In fact, the Oracle backend (or even some higher, more 
generic level) should have doubled those '"' characters to make them part of 
the name. But -- save length issues -- the ploy succeeds:

>  ...
>  (SELECT (1) AS "A"
>   FROM "MNOWOTKA"."PROTEIN_THERAPEFB7C") "_SUB"
>  ...

This is definitely a bug. The correct output should have been

  ...
  (SELECT (1) AS "A"
   FROM "MNOWOTKA"".""PROTEIN_THERAPEFB7C") "_SUB"
  ...

As if this was the 'mnowotka"."protein_therapeutics' table in the current 
schema (wouldn't work -- length 31), and not the protein_therapeutics table 
(way shorter than 30) in the mnowotka schema, which is the intended target.

I'm not sure -- perhaps the db_tablespace option is interpreted by the Oracle 
backend as schema, which would allow what Michal is trying to do with a sane 
API. Otherwise, I'd advise Michal to look at Oracle's table aliases, and mark 
this also as a missing feature.

But either way, not doubling quotes is a bug.

(Michal: you don't need the '\' characters; look up Python quoting). 

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: #16779 - a tutorial for first time Django contributors

2012-11-10 Thread Shai Berger
On Sunday 11 November 2012, Tim Graham wrote:
> 
> I think the part that has the most potential to confuse new contributors is
> the introduction of PYTHONPATH.  Claude suggested we could simply instruct
> users to run the tests like so:
> 
> PYTHONPATH=/path/to/django ./run_tests.py --settings=test_sqlite
> 
> I'm not particularly in love with that, but it would eliminate the need to
> try to explain things

It would leave a lot to explain to Windows users (which I note you are still 
trying to cater for).

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Yet another __ne not equal discussion

2012-11-30 Thread Shai Berger
On Friday 30 November 2012, Marek Brzóska wrote:
> Has the matter been completely put away?
> 
> I would like to bring it up again.
> 
> I have Articles and Categories. An Article belongs to Categories:
> 
> class Category(model):
>   pass
> class Article(model):
>   category = ManyToManyField(Category)
>   status = CharField(max_length=10)
> 
> Now I want all categories with articles that have status different than
> "archived".
> 
> I cannot do this with django's ORM!
> 

live_articles = Article.objects.exclude(status="archived")
live_cats = Category.objects.filter(article__in=live_articles)

Doesn't this do the trick for you?

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Oracle backend bug makes testing on oracle slower?

2012-12-11 Thread Shai Berger
Hi all,

I've just been hit by a very simple and annoying bug on Oracle, on Django 
1.3.4. Without testing, it seems to no longer be as bad on master, but a shade 
of it still remains.

The bug is this: The DatabaseFeatures for the Oracle backend do not specify 
that it supports transactions. 

On 1.3.4, this means the value is only set if confirm() is called, and when it 
is, it only affects a single connection; it does not appear to be called 
whenever a connection is created -- while database test creation calls it, by 
the time the tests use it it is all forgotten and testing is relegated to the 
old behavior of flush for every test.

On master, where the generic supports_transactions is a cached property, this 
means that for every connection opened, 4 statements are being executed to 
check if the database (still) supports transactions.

1. Oracle supports transactions. is there a reason not to just add the line

supports_transactions = True

to its DatabaseFeatures class?

2. Database features, typically, do not change between opening of connections 
within a single server run. Shouldn't we consider saving these features -- 
even the dynamic ones -- on the DatabaseFeatures class, instead of its 
instances?

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Oracle backend bug makes testing on oracle slower?

2012-12-11 Thread Shai Berger
On Tuesday 11 December 2012, Ian Kelly wrote:
> On Tue, Dec 11, 2012 at 6:31 AM, Shai Berger  wrote:
> > Hi all,
> > 
> > I've just been hit by a very simple and annoying bug on Oracle, on Django
> > 1.3.4. Without testing, it seems to no longer be as bad on master, but a
> > shade of it still remains.
> > 
> > The bug is this: The DatabaseFeatures for the Oracle backend do not
> > specify that it supports transactions.
> > 
> > On 1.3.4, this means the value is only set if confirm() is called, and
> > when it is, it only affects a single connection; it does not appear to
> > be called whenever a connection is created -- while database test
> > creation calls it, by the time the tests use it it is all forgotten and
> > testing is relegated to the old behavior of flush for every test.
> > 
> > On master, where the generic supports_transactions is a cached property,
> > this means that for every connection opened, 4 statements are being
> > executed to check if the database (still) supports transactions.
> 
> While connections come and go, the DatabaseWrapper objects are stored
> in the ConnectionHandler on a per-thread basis and should be reused
> for subsequent connections.  So these tests *should* run only once per
> configured connection per thread.  Can you demonstrate that they are
> actually being run on every connection?
> 

As I noted above, I haven't actually tested on master, so I can't be sure; 
this was just my reading of the code. As far as I'm aware, though, the basic 
management of database wrappers and connections hasn't changed much since 1.3, 
and I can demonstrate that on 1.3.4, a new connection uses a new 
DatabaseFeatures object. I found this issue not by optimizing for speed, but 
by using a test-runner which installs some objects in the DB before running 
the test-suite; to my surprise, the objects weren't available, because the 
test framework, thinking the backend didn't support transactions, flushed the 
DB (as mentioned, on 1.3.4 the setting of supports_transactions takes a call 
to confirm(), but one is made, and running tests is single-threaded).

> > 1. Oracle supports transactions. is there a reason not to just add the
> > line
> > 
> > supports_transactions = True
> > 
> > to its DatabaseFeatures class?
> 
> I see no harm in it, although per the above I don't think it will help
> as much as you hope.  Since the test involves a CREATE TABLE though,
> and since DDL is slow in Oracle, and since there's really no guarantee
> that the user even has the necessary permissions to create the table
> in the first place, I think it's a good idea to add this in any case.
> 
Should I open a ticket for it? It is a one-line patch...

> > 2. Database features, typically, do not change between opening of
> > connections within a single server run. Shouldn't we consider saving
> > these features -- even the dynamic ones -- on the DatabaseFeatures
> > class, instead of its instances?
> 
> This would lead to bugs in multi-DB setups where the user has
> configured connections to multiple databases of the same type but
> different versions (and different feature sets).  I don't know how
> common that is, but it is better to be correct and slow than fast and
> buggy.

While I doubt that such setups are common enough to justify a price paid by 
the whole community, I can accept this answer as "we considered it and decided 
against". But then, I would ask for backends to rely as little as possible on 
dynamic features. A user can easily monkey-patch sense into the backend 
DatabaseFeatures, but that's no replacement for sensible defaults.

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Oracle backend bug makes testing on oracle slower?

2012-12-11 Thread Shai Berger
On Tuesday 11 December 2012, Florian Apolloner wrote:
> On Tuesday, December 11, 2012 8:53:55 PM UTC+1, Shai Berger wrote:
> > Should I open a ticket for it? It is a one-line patch...
> 
> Please try to test this on master first, we most likely won't patch 1.3.
> 
This was referring only to the setting of supports_transactions on Oracle, 
which Ian appeared to support. I had no illusions that it would be ported to 
1.3 or even 1.4 (and I have an easy workaround for these versions anyway).

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



A regression that isn't a regression

2012-12-20 Thread Shai Berger
Hi all,

We've encountered a funny bug in our code, which I thought was worth sharing. 
I don't think a bug in Django is involved, but I suspect Django could do 
better to prevent it.

The issue is with the distinct() method of querysets. Up to Django 1.3, it was 
documented as taking no arguments; in Django 1.4, an optional "*fields" 
argument was added, to support Postgresql's "DISTINCT ON" feature.

However, in Django 1.3, distinct() did take one argument: 

def distinct(self, true_or_false=True):
  ...

Some of our coders, who apparently only glimpsed at the documentation, made 
calls to distinct with one field:

SomeModel.objects.filter(...).distinct('field')

This worked as if distinct() was called with no arguments, because 'field' 
evaluates True; it broke with the move to Django 1.4, because we don't use 
Postgresql.

Now, of course, Django is not to blame for developers who only read the titles 
of the documentation (and documentation for the wrong Django version, to 
boot); but I still claim it is a minor gotcha, because the *fields argument was 
on the /dev/ version docs for a long time, and many times it is those pages 
that come up first in web searches.

I was thinking along the lines of adding,  in the 1.3 branch, when the change 
was made (that is, in the days before 1.4), some warning for anyone using the 
old, undocumented, soon-to-be-gone API; something along the lines of

def distinct(self, true_or_false=True):
   
if true_or_false is not True:
   warn(DeprecationWarning, "this API is about to change")

The issue for this call is moot now, but there may be others like it.

Thoughts?

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: A regression that isn't a regression

2012-12-20 Thread Shai Berger
On Thursday 20 December 2012, Karen Tracey wrote:
> This has come up before, see: https://code.djangoproject.com/ticket/17974
> 
Figures...

> 1.3 is security-fix only at this point, 

Yes, and pretty close to not-even-security-fix; I'm well aware of that, and 
that's why I wrote:

> > The issue for this call is moot now, but there may be others like it.

and...

> we'd have to make a pretty big
> exception to put anything into 1.3 to warn users of their mis-use of an
> undocumented parameter to distinct
> 
I didn't think this was a valid bug. I was trying to make the point that, in 
general, 

* when intorducing a call in the development branch,
* which, if used mistakenly on a maintenance branch, would "work",
* that is, not do the new functionality, but not raise an exception,
* then it is worthwhile to put a warning in the maintenance branch.

Sorry if I wasn't clear,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Relative path support for TEMPLATE_DIRS and others in settings.py (django ticket 694)

2012-12-22 Thread Shai Berger
Hi,

On Saturday 22 December 2012, Florian Apolloner wrote:
> On Saturday, December 22, 2012 10:35:59 PM UTC+1, Ben Porter wrote:
> > I would like to see support for relative paths.  It seems the solution is
> > simple, but I wonder if there is some compelling reason to require
> > absolute paths?
> 
> It would seem so but it is everything but simple: First of, for a relative
> path one needs a base path to join with, what is that? In your example it's
> the project root, Django doesn't have such thing as a project root.

You are right, but that might be the root of the problem (no pun intended). 
Django doesn't have a concept of a project root, and (partly as a result) 
requires paths to almost anything that isn't a python module to be given as 
absolute path.

I think adding an optional "PROJECT_ROOT" or "PROJECT_PATH_BASE" setting, 
specifying that other paths can be made relative to it, will remove a 
significant amount of boilerplate in settings files, and will not have any of 
the problems you name.

Personally, I have yet to see a settings file in a non-trivial Django project 
that doesn't do the os.path.join(os.path.dir(__file__), ...) dance. When done 
properly, you end up with a function in_proj_dir that is applied to many 
settings values. It would be much cleaner, IMO, to have this code defined in 
the framework (rather than settings files) and limit the use of code to a 
single setting.

My 2 cents,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Oracle testing issue, looks like bug: Separate connections to same DB

2012-12-27 Thread Shai Berger
Hi all,

I'm seeing a problem with testing on Oracle, in a setting where there are two 
defined databases that reference the same connection parameters (this is done 
to enable operations in two parallel, separate transactions); the 'other' 
database is not treated as a test database. This means that, during tests, 
operations on it are performed on the 'production' database (this is all in a 
development environment, thankfully).

Has anyone else run into this? 

Django==1.4.3

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Oracle testing issue, looks like bug: Separate connections to same DB

2012-12-27 Thread Shai Berger
Hi Jani and all,

On Thursday 27 December 2012 13:28:23 Jani Tiainen wrote:
> 27.12.2012 13:08, Shai Berger kirjoitti:
> > Hi all,
> > 
> > I'm seeing a problem with testing on Oracle, in a setting where there are
> > two defined databases that reference the same connection parameters
> > (this is done to enable operations in two parallel, separate
> > transactions); the 'other' database is not treated as a test database.
> > This means that, during tests, operations on it are performed on the
> > 'production' database (this is all in a development environment,
> > thankfully).
> > 
> > Has anyone else run into this?
> > 
> > Django==1.4.3
> > 
> > Thanks,
> > 
> > Shai.
> 
> I assume that you're referring to test setup instructions on
> https://code.djangoproject.com/wiki/OracleTestSetup page?
> 

Yes and no: I am referring to a variation on this theme.

> You should see that there is defined TEST_USER, TEST_TBLSPACE and
> TEST_TBLSPACE_TMP variables to different names since those are ones used
> when running tests.
> 

In my case, these values *should* be equal. I am not trying to run the Django 
test suite, but tests for my own applications, and my applications rely on 
having two separate connections to the same database.

> Django uses main user just to [...]

I understand pretty well how the django test framework uses its connections. 
I'm pointing out a case where it isn't doing it correctly.

To clarify: I am defining my databases by something like this in settings:

DATABASES = {
   'defailt' : ... # some full definition
}

DATABASES['auditlog'] = DATABASES['default'].copy()

and I even add this, which should help

DATABASES['auditlog']['TEST_MIRROR'] = 'default'

but when I run the tests, and print out the configuration from the test runner, 
it looks like this:

{'DATABASES': {'auditlog': {'ENGINE': 'healarium.utils.oracle_backend',
'HOST': 'oracle',
'NAME': 'orcl',
'OPTIONS': {'threaded': True},
'PASSWORD': '',
'PORT': '1521',
'TEST_CHARSET': None,
'TEST_COLLATION': None,
'TEST_MIRROR': 'default',
'TEST_NAME': None,
'TEST_USER': 'test_usr1',
'TIME_ZONE': 'Asia/Jerusalem',
'USER': 'usr1'},
   'default': {'ENGINE': 'healarium.utils.oracle_backend',
   'HOST': 'oracle',
   'NAME': 'orcl',
   'OPTIONS': {'threaded': True},
   'PASSWORD': 'Im_a_lumberjack',
   'PORT': '1521',
   'SAVED_PASSWORD': '',
   'SAVED_USER': 'usr1',
   'TEST_CHARSET': None,
   'TEST_COLLATION': None,
   'TEST_MIRROR': None,
   'TEST_NAME': None,
   'TEST_USER': 'test_usr1',
   'TIME_ZONE': 'Asia/Jerusalem',
   'USER': 'test_usr1'},

The thing to notice is how in 'default' there is a 'SAVED_PASSWORD' and 
'SAVED_USER', and the 'USER' is equal to the 'TEST_USER', while in 'auditlog' 
(the 'other' database connection) these do not hold (the only reason 
'auditlog' has a correct 'TEST_USER' entry is that in trying to track this 
down, I defined it explicitly for 'default' and it was copied).

Hope the issue is now clearer,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Oracle testing issue, looks like bug: Separate connections to same DB

2012-12-27 Thread Shai Berger
On Thursday 27 December 2012 15:11:31 Jani Tiainen wrote:
> 
> TEST_MIRROR = 'default'  means that instead of creating test database of
> "auditlog" using independent database connection Django will reuse
> connection to "default". That's how Django makes testing of replication
> happen (that's what TEST_MIRROR setting is for).
> 

Removing TEST_MIRROR changes nothing (except the printing of databases). If 
you were correct, and TEST_MIRROR were the issue, then this would cause the 
whole test suite to fail on attempt to create the 'auditlog' test database 
(which would already exist); but no attempt is made to create this database.

Also, my tests currently fail because they include some 
'model.objects.get(...)' call, which would succeed if the test were run on an 
empty database; instead, it fails because it retrieves more than one record. 
The number of records returned increases each time I run the test, proving (to 
me, at least) that the records are written to a persistent database, not one 
that is deleted when the tests end.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Oracle testing issue, looks like bug: Separate connections to same DB

2012-12-27 Thread Shai Berger
On Thursday 27 December 2012 15:16:15 Jani Tiainen wrote:
> 
> Just out of curiosity - what's the rationale to duplicate connection
> information? Why it's so important?

As I noted before, the idea is to use the second connection to write things in 
a separate transaction. As the name 'auditlog' implies, this is an audit log; 
I don't want records written there to be rolled back when a request encounters 
an error (one of these records may well include the exception).

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Oracle testing issue, looks like bug: Separate connections to same DB

2012-12-27 Thread Shai Berger
Hi again,

On Thursday 27 December 2012, Jani Tiainen wrote:
> 27.12.2012 15:32, Shai Berger kirjoitti:
> > On Thursday 27 December 2012 15:16:15 Jani Tiainen wrote:
> >> Just out of curiosity - what's the rationale to duplicate connection
> >> information? Why it's so important?
> > 
> > As I noted before, the idea is to use the second connection to write
> > things in a separate transaction. As the name 'auditlog' implies, this
> > is an audit log; I don't want records written there to be rolled back
> > when a request encounters an error (one of these records may well
> > include the exception).
> 
> I've never done such a setup (we always have separate schemas) so I've
> feeling that testing such a setup doesn't work right away.
> 
> One option could be create separate test users and add synonyms (by
> custom SQL script on auditlog test database creation) for auditlog user
> to default test user auditlog tables.
> 

That is one ugly workaround, but I guess I'll take it if I can't find anything 
else.

> There also exists a oldish ticket:
> https://code.djangoproject.com/ticket/14415
> 
> That indicates that issue is fixed...

Yes, I submitted that ticket. Problems there were much worse than the problems 
I'm facing now, but it is all around the same issue. And BTW, the behaviour on 
1.3.5 is the same as 1.4.3 (not that I expect 1.3 to be fixed for this, just 
indicating this is not a 1.4-era regression).

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Relative path support for TEMPLATE_DIRS and others in settings.py (django ticket 694)

2012-12-28 Thread Shai Berger
On Saturday 29 December 2012, Cal Leeming [Simplicity Media Ltd] wrote:
> Since the day I started using Django, I have always used a relative path
> for TEMPLATES_DIR.
> 
> import os
> CURRENT_DIR = os.path.realpath(os.path.dirname(__file__))
> TEMPLATE_DIRS  =  "%s/templates/" % ( CURRENT_DIR, )
> 
The main point of the idea was to enable relative paths without the string 
manipulations, that is,

import os
PROJECT_ROOT = os.path.realpath(os.path.dirname(__file__))
TEMPLATE_DIRS  =  ["templates/"]

It has some merit because TEMPLATE_DIRS is not the only such setting, so 
adding the project root to a path is a pattern repeated several times in the 
settings file.

> Imho, the idea of having to hard code your PROJECT_ROOT is ludicrous.
> 
Nobody suggested any such thing (well, except, maybe, the comments in the 
current default project template).

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Oracle testing bug (was Re: Oracle testing issue, looks like bug: Separate connections to same DB)

2012-12-30 Thread Shai Berger
Hi again,

On Thursday 27 December 2012 15:50:58 Jani Tiainen wrote:
> There also exists a oldish ticket: 
> https://code.djangoproject.com/ticket/14415
> 
> That indicates that issue is fixed...

After looking further into this, this is indeed a bug, and indeed separate 
from 14415. For one, the problem now is only with Oracle.

The root of the problem is that unlike the other DBMSs, where you can create a 
new database for testing, Oracle does not have the concept of separate 
databases within the same server. So, to have a clean schema to work in, the 
test framework creates a new user (in Oracle there's also a 1-1 correspondence 
between users and schemas) when it creates the test database. 

This is all done in the backend-specific _create_test_db(). The problem is that 
this code only takes care of a single db (naturally). There is generic code 
(in DjangoTestSuiteRunner.setup_databases()), that takes care of multiple dbs 
-- including same-db-references and mirrors (hereafter, for simplicity, just 
"mirrors"). This code isn't aware of the Oracle-specific settings changes. To 
handle mirrors, it just updates the database name of the mirror to the one set 
by _create_test_db(). In the Oracle case, this is a no-op 
(self.connection.settings_dict['NAME'] is not changed); and nobody is left to 
update the user, password and tablespace parameters, which the backend does 
change.

This means that, not just my somewhat peculiar use case is affected -- the 
whole concept of TEST_MIRROR is broken on Oracle.

Can anyone attest differently? Is there a reason not to open a bug?

Also, I intend to fix this by using a custom test-runner, which will invoke a 
new backend-specific API to take care of the mirroring; the default behavior 
will be the one used today, and I'll be able to override it in (my custom) 
Oracle backend.

Any comments about this approach?

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Oracle testing bug (was Re: Oracle testing issue, looks like bug: Separate connections to same DB)

2012-12-31 Thread Shai Berger
On Sunday 30 December 2012 21:54:52 Anssi Kääriäinen wrote:
> 
> Seems like you are correct [...]. Please open a bug.
> 
For anyone interested that isn't on the new-bugs list, it's 
https://code.djangoproject.com/ticket/19542

For anyone else: my apologies for the noise.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Relative path support for TEMPLATE_DIRS and others in settings.py (django ticket 694)

2013-01-01 Thread Shai Berger
Hi,

I have to take back my support of PROJECT_ROOT, in view of Aymeric's 
arguments. However, now I think he isn't pursuing the conclusions of the 
argument far enough:

On Tuesday 01 January 2013, Aymeric Augustin wrote:
> For instance, instead of using TEMPLATE_DIRS, project-wide templates can go
> into an "myproject" application. 
 
"There should be one-- and preferably only one --obvious way to do it". If a 
project app is the way to do it, TEMPLATE_DIRS should be deprecated. 
Conversely, at the moment, a project app is the non-obvious way to do it (and 
it carries with it further non-obviousness, like the importance of the order 
of INSTALLED_APPS).

Also, settings with essentially the same behavior:
FIXTURE_DIRS
LOCALE_PATHS
STATICFILES_DIRS (mentioned in contrib.staticfiles docs, but not main settings 
doc)

> There are two special cases that don't fit into apps: STATIC_ROOT and
> MEDIA_ROOT. (Technically, there's ALLOWED_INCLUDE_ROOTS too, but it isn't
> commonly used.)
> 
Also:
In CACHES: LOCATION when BACKEND is FileBasedCache
In DATABASES: NAME when ENGINE is sqlite3 (relative path accepted here; 
granted, sqlite3 is not recommended for production)
FILE_UPLOAD_TEMP_DIR
SESSION_FILE_PATH (for file-based sessions)

Third-party apps may add further such paths, where working data gets stored.

[Good arguments, about best practices for production and how paths relative to 
the sources' root are only acceptable for development, snipped]

> For these reasons, I'm against codifying PROJECT_ROOT and relative paths in
> Django's default settings files.

But what if we called it "WORK_FILES_ROOT" (or something similar), and made it 
clear that it's expected to lie outside the sources?

> No amount of "+1 because I'm doing it already" or blog posts will make it a
> best practice in production.
 
One point to take from those posts and reported practices, is that your 
current position does not promote the cause of keeping working-data (and work-
copies of static files) out of the source tree. instead, it promotes 
boilerplate coding in settings files, and a dangerous mixing of non-python 
source files with work-files.

I think we can (and should) do better -- deprecate the global *_DIRS settings, 
include a project app in the default settings (at least as a comment), clarify 
the importance of order in INSTALLED_APPS, and make putting the work-files out 
of the sources obvious.

My 2 cents,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Minor feature: Make TestCase pass the testcase instance to self.client's constructor

2013-01-05 Thread Shai Berger
On Friday 04 January 2013, Malcolm Box wrote:
> 
> The general pattern I want to implement is have a test client that makes
> assertions about all the requests made during a set of tests. For example,
> it could check that every get() returned cache headers, or that
> content_type is always specified in responses. Or that the response has
> certain HTML, uses certain templates etc - ie all of the assertion testing
> goodness in django.test.TestCase.
> 
> My concrete use case is a JSONClient that adds a json_get / json_post
> methods which makes sure to setup content_type etc on the call, and then
> validates that what came back was valid JSON.
> 
> The simple, wrong way is to do:
> 
> def check_response(self, response):
>self.assertContains(response, )
>
> 
> def test():
>r = self.client.get(...)
>self.check_response(r)
> 
> but this is error prone, verbose etc etc.
> 
> The right thing is that within a test suite, the get()/post() etc to do the
> checks for me - and so it should be possible to create a testclient that
> does this, and be able to use this testclient in various test suites.
> 

No, you're mixing concerns.

> Is there a simple, clean way to solve this that I'm missing?
> 

class MyTestCase(TestCase):

def check_response(self, response):
self.assertContains(response, )

def checked_get(self, *args, **kw):
r = self.client.get(*args, **kw)
self.check_response(r)
return r

class SpecificTest(MyTestCase):

def test():
r = self.checked_get(...)
...

That's how I would do it.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Minor feature: Make TestCase pass the testcase instance to self.client's constructor

2013-01-05 Thread Shai Berger
Hi again Malcolm,

I think making assertions in the test client is wrong for most situations, 
though I can't (of course) say much about your specific case; so I'm -1 on 
adding the testcase as a client initializer argument (this would send the 
message that such things are encouraged).

However, note that the only use of the client_class attribute in the Django 
code is in the simple invocation:

self.client = self.client_class()

which means you can achieve your goal, with no change to Django code, using 
this (admittedly, a little abusive) hack:

class MyTestBase(TestCase):

def client_class(self):
return MyTestClient(self)

If your own code refers to self.client_class and expects to have access to its 
attributes, you can still arrange it with a little more effort (an elaborate 
and transparent way, using a descriptor, and a simple way, using another 
attribute, are both left as an exercise to the reader).

HTH,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Reminder: pending Oracle fixes and issues

2013-01-17 Thread Shai Berger
Hi Django devs, and in particular Oracle users,

This is to remind you of fixes I proposed for the Oracle backend, which were 
mostly discussed and accepted, but seem not to have made it into the codebase 
yet. I am referring to:

- PR #393[0] - Includes two separate issues -- of which one has been handled 
by including the specific fix (% signs in table names), and the other is, to 
the 
best of my knowledge, is yet unhandled (very slow processing of numbers). This 
issue has been discussed at some length with Ian and Anssi before the 1.5 
feature freeze, and was then defined as a bug that can get in later -- but it 
didn't (I'm not sure if it is severe enough to justify inclusion now that an 
RC has been released, but I'd certainly want to see it in master).

- PR #411[1] - which fixes #10070[2] for Oracle. I don't even know if this fix 
is ready for checkin or not -- I know it fixes a bug that appears to be 
accepted, but the bug is about an undocumented feature (which is, 
nevertheless, available for other backends). I haven't added documentation or 
tests; my question on the bug about this was left unanswered. While I did make 
the PR before the feature freeze, I have no expectations of this getting into 
1.5 -- but I would like to see it go in sometime...

Besides these two, I would like to continue the discussion on PR #402[3]; it 
suggests adding a flag to Oracle's DATABASE_OPTIONS, to control how it returns 
explicit expressions from raw sql ("select 1.5 from ..."). The current backend 
(also after inclusion of PR #393) returns a python Decimal, which is the Right 
Thing(tm) to do for Oracle, but breaks cross-database compatibility (Decimals 
don't mix well with floats). In the previous discussion[4], Anssi essentially 
rejected the cross-database issue, asserting that this needs to be handled 
query-by-query, and by implication, on a backend-specific basis; I think that 
requiring reusable apps to include backend-specific code, when they use common, 
standard sql; and with no simple fix for the user, is the worse choice here 
(although the user can do what I did, and use a customized backend).

Thanks for your attention,

Shai.

[0] https://github.com/django/django/pull/393
[1] https://github.com/django/django/pull/411
[2] https://code.djangoproject.com/ticket/10070
[3] https://github.com/django/django/pull/402
[4] https://groups.google.com/d/msg/django-developers/4BNkJyGez9A/1yBVs-NL2h0J

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Reminder: pending Oracle fixes and issues

2013-01-19 Thread Shai Berger
On Thursday 17 January 2013, Anssi Kääriäinen wrote:
> On 17 tammi, 15:50, Shai Berger  wrote:
> > Besides these two, I would like to continue the discussion on PR #402[3];
> > it suggests adding a flag to Oracle's DATABASE_OPTIONS, to control how
> > it returns explicit expressions from raw sql ("select 1.5 from ...").
> > The current backend (also after inclusion of PR #393) returns a python
> > Decimal, which is the Right Thing(tm) to do for Oracle, but breaks
> > cross-database compatibility (Decimals don't mix well with floats). 
> > 
[...]
> 
> For the compatibility issue: both MySQL and PostgreSQL return Decimal
> for "select 1.5". SQLite returns float. If you want to write cross-db
> compliant raw SQL you have to deal with leaky abstractions.
> 
Indeed they do. I was under the impression that it's only Oracle that does 
(the other backends I usually "cross" to are Sqlite and the 3rd-party SQL 
Server via pyodbc, and both return float). This makes my original motivation 
completely misguided – I was thinking that people will be writing apps and not 
run into the issue, because so few (at least relatively) use Oracle; that the 
only people who would expect a Decimal in this situation would be already 
working with Oracle, and find out about the problem soon enough. I was wrong.

So I agree with Anssi, and I closed PR #402.

Which still leaves two issues -- PR #393 (faster processign of numbers; I 
think I forgot to mention earlier -- it is just some fixes on work by Ian 
Kelly), and PR #411 (named parameters).

Per Jani's PR#650 (which I just went to review and found already accepted), I  
note that the fix in #393 requires cx_Oracle 5.0.1; if it is accepted, the 
check for cx_Oracle major version >=5 can be removed (the requirement change 
comes from Ian's modifications, so it is apparently acceptable at least to one 
core developer).

Thanks for your attention,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Oracle testing bug (was Re: Oracle testing issue, looks like bug: Separate connections to same DB)

2013-01-19 Thread Shai Berger
On Thursday 03 January 2013, Jani Tiainen wrote:
> 31.12.2012 14:18, Shai Berger kirjoitti:
> > On Sunday 30 December 2012 21:54:52 Anssi Kääriäinen wrote:
> >> Seems like you are correct [...]. Please open a bug.
> > 
> > For anyone interested that isn't on the new-bugs list, it's
> > https://code.djangoproject.com/ticket/19542
> > 
> > For anyone else: my apologies for the noise.
> > 
> > Shai.
> 
> Can you please setup simple testcase to ease bugfixing?

(sorry about the delay in responding to this -- mis-managed mail)

The issue here is mostly to do with settings; thus, no testcase could be added 
to the Django test suite. There are similar comments from Russel Keith-Magee 
quoted on the old bug #14415.

I couldn't reasonably provide a working settings file for Oracle; for debugging 
this, you just need to have a file with two connections, and have one marked as 
a test mirror, i.e.

DATABASES = {
'default' : {
... whatever
},
'other' : {
... whatever
'TEST_MIRROR' = 'default'
}
}

Test code for this should look like (untested):

from django.contrib.auth.models import Group

class MirrorTest(TransactionTestCase):
  def test_method():
gname='unique_and_unnatural'
g1 = Group.objects.create(name=gname) # uses default
transaction.commit(using='default')
g2 = Group.objects.all().using('other').get(name=gname)
self.assertEqual(g1,g2)

The test should verify that 'default' and 'other' are indeed the same db.

Hope this helps,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: [SPAM] [New feature] What do you guys thinks about an automatizated ER model drawer?

2013-01-25 Thread Shai Berger
Hi,

On Friday 25 January 2013, Vitor Lima wrote:
> 
> I was thinking in something like a feature to django-admin that will make
> an automatic ER(entity-relationship) model of the entire database. I think
> that this will help with documentation and with the projects that don't
> have an ER model.
> It'll draw something like that:
> http://www.memoireonline.com/10/10/4048/Design-and-implementation-school-ma
> nagement-system25.png
> 
> What do you guys think?
> 
It has been done in third-party applications; see
http://packages.python.org/django-extensions/graph_models.html
and https://code.djangoproject.com/wiki/DjangoGraphviz.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: db_type method for ForeignKey is buggy?

2013-01-30 Thread Shai Berger
Hi Michał,

On Monday 28 January 2013, Michał Nowotka wrote:
> 
> This is db_type method definition:
> 
> def db_type(self, connection):
> rel_field = self.rel.get_related_field()
> if (isinstance(rel_field, AutoField) or
> (not connection.features.related_fields_match_type and
> isinstance(rel_field, (PositiveIntegerField,
>PositiveSmallIntegerField:
> return IntegerField().db_type(connection=connection)
> return rel_field.db_type(connection=connection)
> 
> What are the consequences of defining it like this?:
> 
>  def db_type(self, connection):
> rel_field = self.rel.get_related_field()
> return rel_field.db_type(connection=connection)
> 
> It shouldn't break anything but it gives an ability to define custom
> auto fields.
> Is the original code buggy or I'm just missing somthig?

I suspect you're missing that, on some backends (notably MySql), AutoField is 
not just an integer column, but a database-supported identity. That type 
cannot, in general, be used for the foreign key pointing at the PK.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Form.set_data method

2013-01-30 Thread Shai Berger
On Thursday 31 January 2013, Byron Ruth wrote:
> Here is the ticket: https://code.djangoproject.com/ticket/19668 and the
> pull request https://github.com/django/django/pull/674
> 
> One user commented on the ticket raising a concern that it could possibly
> be misused if the data is set after it had been used. It is certainly a
> valid concern, however it should be made clear in the docs when to use it
> and/or raise an exception if `is_valid` has already been called.
> 
> Thoughts?

While this is backwards-compatible per se, using it in views is generally not 
backwards-compatible with user form classes (you can't tell what they do in 
their initializers); thus, generic views (and also some not-generic views) are 
forced to keep using the "old way" unless the (user) form code is altered. 
Which means, you'll have two ways to do the same thing (in views), without a 
clear preference between them.

So if you want this judged as a backwards-compatible change, I'm -1.

As a non-backwards-compatible change, I'd like it, but I don't think it's 
worth the disruption, so I'm -0.

Either way, I'm not a core dev.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: cycle in nested loops

2013-02-02 Thread Shai Berger
Hi,

On Friday 01 February 2013, Aymeric Augustin wrote:
> Hi Wim,
> 
> Le 21 janv. 2013 à 18:01, Wim Feijen  a écrit :
> > Actually, the current behaviour is not to reset between loops. Is that
> > intentional?
> 
> Yes, this behavior is intentional. See Malcolm's comment (#4) on the
> ticket.
> 
> > A patch to ticket 5908 adds a reset_cycle tag, which resets the
> > cycle-loop. I am wondering if it is the right way to solve this problem
> 
> That's what Malcolm said, and I tend to agree with him. I'm not
> particularly excited by adding a new tag but I don't have a better idea.
> Something like {% cycle row_colors reset %} would conflict with the
> regular {% cycle %} syntax.
> 

There are two other ways I see.

1) There is a way to include a 'reset' keyword in the cycle tag -- it only 
requires the use of an assignment; the same way allows use of the 'silent' 
keyword.

{% cycle 'v1' 'v2' v3 as dummy reset %}

This is a little awkward -- the 'silent' keyword indeed doesn't make sense 
without an assignment, but for 'reset' forcing the assignment is only done for 
backwards-compatibility's sake. Also, as Aymeric said,

> Besides, introducing coupling between {% cycle %} and 
> {% for %} / {% endfor %} sounds like a bad idea.

2) I think it may be better to solve this by giving the user more control over 
the cycling -- letting them set the index variable, so, use something like

{% cycle 'v1' 'v2' v3 index=forloop.counter0 %}

Where this still deserves being called "cycle" and not "select" because of the 
cyclic interpretation of the index. This solves the problem in the ticket, and 
allows other possibilities as well.

The syntax is new, so it doesn't collide with old syntax in the cycle tag. 
Named arguments like this are not used often in template tags, but they do 
exist in the {% include %} tag (admittedly, there is a reason for it there 
that is not present here). Other alternatives which I see as acceptable are to 
use one of the keywords used in other tags -- 'on' (used in autoescape) or 
'by' (used in regroup) seem appropriate:

{% cycle 'v1' 'v2' v3 by forloop.counter0 %}

This is strictly not backwards-compatible, as it will break templates where a 
context-variable named 'by' was used as an argument to the cycle tag, but I 
suspect that may be acceptable.

Anyway, my 2 cents,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Form validation - char field coerces *any* data into a unicode

2013-02-09 Thread Shai Berger
On Saturday 09 February 2013, Yo-Yo Ma wrote:
> A form that has a char field (e.g. "name") when provided a dict of data
> will convert the value of "name" to a Unicode, no matter what. I understand
> that this is desirable in some circumstances, but it can lead to things
> 
> like:
> >>> product.name
> 
> u"{'haha': 'I am a dict'}"
> 
> Perhaps this is desirable, but I wonder if there is any merit to the idea
> of sanitizing data to ensure it is "valid" for a char field, since
> practically *any* Python object can be cast into a Unicode (vs a
> DecimalField or an IntegerField, which will raise an error).
> 
> I realize the distinction of a "valid" would be completely arbitrary (e.g.,
> who's to say that a dict isn't a valid char field value, but an that
> integer is?), but nonetheless, here I am, requesting feedback.

I think an important point to keep in mind is that it isn't just 'dict', but 
also ArbitraryUserClass which is considered valid today, and losing this will 
break many users' code. So I'm -1 on adding such validation to the core 
CharField.

There may be merit to such validation in special cases -- for this, it is easy 
enough to subclass CharField and override its to_python() method (that's the 
one that converts the value to unicode). Something along the lines of

class StrictCharField(CharField):
def to_python(self, value):
if isinstance(value, basestring):
return super(StrictCharField, self).to_python(value)
else:
raise TypeError('Get off my lawn')

(this is untested python-2-only pseudocode, YMMV).

My 2 cents,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: select_for_update running on db_for_read

2013-02-19 Thread Shai Berger
Hi,

On Monday 18 February 2013, Ioan Alexandru Cucu wrote:
> 
> If I'm running a select_for_update statement in a multidb environment that
> uses a read-only slave database
> 
Why does this operation make sense?

Why would you select-for-update if you don't intend to update? Or does 
updating on a read-only slave somehow make sense?

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Database pooling vs. persistent connections

2013-02-21 Thread Shai Berger
Hi,

On Sunday 17 February 2013, Aymeric Augustin wrote:
> **tl;dr** I believe that persistent database connections are a good idea.
> Please prove me wrong :)
> 
> [...]
> 
> So -- did I miss something?

I think you haven't -- but several points came to mind, which weren't 
discussed in this thread. As far as I can tell, none of them are problems, but 
*I* may be missing something, so I thought I should raise them here.

First -- the very mention of this topic reminded me of 
https://code.djangoproject.com/ticket/9964, which was my pet bug for about two 
years. The point of that bug, though, was to make sure that transactions are 
properly closed before (and regardless if) the connection is closed, so while 
related, it should be unaffected.

Second -- Tasks out of requests. Core developers participating in this 
discussion already have this in mind, Aymeric has commented about it in 
https://code.djangoproject.com/ticket/17887, but it was left out of the 
discussion on the thread. The suggested changes, AFAICT, modify the behavior 
around the end of requests only -- for tasks, nobody closed the connection 
before, and nobody is going to do anything different now; so that's not 
"something missed" either.

Third -- different use patterns of multi-db. AFAICT the change treats all 
connections as equals, so no new problems should arise, but I can't remove a 
gnawing suspicion that some interaction may pop up. In particular, I'm worried 
because not all such patterns are automatically tested; I know my pet pattern 
(two aliases for one db, for different transaction behavior -- for logging into 
db, which shouldn't be reverted when the main transaction is rolled back) has 
had problems unnoticed by others (see e.g.  
https://code.djangoproject.com/ticket/14415).

And last (and probably least) -- coordinated distributed transactions and two-
phase-commit are not really supported by Django, and if you want them you need 
to write backend-specific code (probably a custom backend -- I haven't had the 
[mis]fortune to need to do this yet). I suspect such code would be affected, 
I'm not sure if it becomes easier or harder to write. I know the issue is 
mostly theoretical, and even if there is a problem with it, that's not a 
blocker; still, I thought it was worth a mention.

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Is there a buildbot? Is there a waterfall? Do the tests pass against all backends?

2013-02-22 Thread Shai Berger
On Sunday 17 February 2013, Florian Apolloner wrote:
> Hi,
> 
> you can see the tests at http://ci.djangoproject.com/ -- currently Oracle
> is untested cause it's a major pita to setup on ubuntu.
> 

The easiest way I know to set up Oracle on Debian/Ubuntu is to use Oracle's 
"Dev Days" VM appliance. It places non-trivial hardware requirements (about 
1.5G memory just to be able to run the VM under virtualbox -- it is set up to 
ask for 1G "internally" and VB has its own overhead), but other than that, it 
was quite easy to get going. I use it on my laptop (which is easily strong 
enough) for development.

http://www.oracle.com/technetwork/community/developer-vm/index.html

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [SPAM] Re: Is there a buildbot? Is there a waterfall? Do the tests pass against all backends?

2013-02-24 Thread Shai Berger
On Sunday 24 February 2013, Florian Apolloner wrote:
> 
> Yeah, I know about it and set it up on the CI during the sprints, it's
> still segfaulting somewhere :/
> 
> http://ci.djangoproject.com/job/Django%20Oracle/database=oracle,python=pyth
> on3.3/12/console
> 
Mybe it's me -- I couldn't get to any report saying what the problem was... 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Is there a buildbot? Is there a waterfall? Do the tests pass against all backends?

2013-02-25 Thread Shai Berger
On Monday 25 February 2013 13:41:10 Florian Apolloner wrote:
> The problem is that it segfaults, when something like a segfault happens
> you usually don't get more information than that... I tried to debug the
> segfault but cx_Oracle or rather the instantclient stuff is installed
> without debug information :/

Without knowing anything, my first guess would be a mismatch between the python 
version used to run the test and the one used to build cx_Oracle. But if you 
install cx_Oracle from source, that's probably not the issue...

Regardless, as far as I could see, the tests were only attempted for Python3 
versions. Did you try to set it up for Python2?

[I hope I'm not coming off pushy -- I only want to help, and will be happy to 
get my hands dirty with the actual setup].

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [SPAM] Re: Is there a buildbot? Is there a waterfall? Do the tests pass against all backends?

2013-02-25 Thread Shai Berger
Hi Florian,

On Tuesday 26 February 2013, Florian Apolloner wrote:
> On Monday, February 25, 2013 5:27:06 PM UTC+1, Shai Berger wrote:
> > the tests were only attempted for Python3
> > versions. Did you try to set it up for Python2?
> 
> No, since the Oracle tests are somewhat slow we decided to just test one
> Python for now. I will try to see if Python 2 makes a difference, didn't
> yet think of it.
> 

Cool.

> > [I hope I'm not coming off pushy -- I only want to help, and will be
> > happy to get my hands dirty with the actual setup].
> 
> Oh sorry, didn't mean to give you that feeling,

No, I was just being extra careful -- I suspected I was beginning to "back-
seat drive".

> it would be of great help
> if you could setup cx_Oracle on an Ubuntu 11.10 (I have to double check
> that tomorrow, don't have my ssh keys with me currently) 64 bit system with
> python 3.3 and see if you can get the tests running or if you also get a
> segfault.
> 

I'll try, it will probably take me a few days though. My regular system is 
Debian Testing.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Decision needed on formfield_callback

2013-02-27 Thread Shai Berger
On Wednesday 27 February 2013, poswald wrote:
> I'm wondering if we can open a quick discussion on the `formfield_callback`
> method of django ModelForms..
> 
> The basic issue boils down to the fact that we have an undocumented (and
> thus unsupported) method on ModelForm that would benefit greatly from a
> small change in behavior and an official blessing in the form of some
> documentation:
> 
> https://code.djangoproject.com/ticket/12915
> 
> This would be very helpful to developers who want to ensure that all of the
> model forms extending their form get a certain overridden field, or for a
> project like django-floppyforms. In the ticket Russ mentions not being 100%
> happy with the patches. Is there a better way to define the standard field
> type to use for a given type?

I took a glance, and it seems like the formfield_callback belongs in the opts 
object (and the Meta inner class), not the form class itself.

My 2 cents,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [SPAM] Re: Is there a buildbot? Is there a waterfall? Do the tests pass against all backends?

2013-02-27 Thread Shai Berger
Hi Florian,

On Wednesday 27 February 2013, Florian Apolloner wrote:
> Hi Shai,
> 
> On Tuesday, February 26, 2013 2:18:14 AM UTC+1, Shai Berger wrote:
> > > No, since the Oracle tests are somewhat slow we decided to just test
> > > one Python for now. I will try to see if Python 2 makes a difference,
> > > didn't yet think of it.
> > 
> > Cool.
> 
> They do work on python2!

Double cool!

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: first() and last(), earliest() and latest()

2013-02-27 Thread Shai Berger
On Thursday 28 February 2013, Aymeric Augustin wrote:
> 
> I would support renaming them to first / last through a deprecation path.
> The new aliases would be available immediately, the old ones would be
> removed in two versions.
> 
+1

> And while we're there, I suggest to rely on the existing "ordering" meta
> attribute and to remove "get_latest_by". I suspect that in many cases
> these two attributes have the same value, and you can specify an explicit
> ordering otherwise.

Consistent with the above, +1

and as far as Wim's original question is concerned:

> Which style do you prefer?
> 
> .filter(last_name__startswith='b').order_by('last_name').first()# clear
> and long
> .first(last_name__startswith='b').order_by('last_name')# first method
> has filter syntax.
> .filter(last_name__startswith='b').first('last_name')   # first method has
> order_by syntax.

ordering is given by position, filtering by keyword arguments -- why not 
support both?

def first (self, *ordering, **filtering):
...

My only concern is one that Anssi raised -- the check for multiple objects is 
discarded, and there is no convenient way to get 0-or-1 objects (which is the 
semantics of the try-get-except-DoesNotExist-return-None pattern this is 
designed to replace). I don't think it has been given enough attention in the 
discussion so far.

One option -- with my suggested syntax -- is that if no ordering is given, 
then it means only one is expected (and, say, None could be used to say "use 
default ordering"). I suspect, though, that this might be a confusing (and 
conflated) interface.

Or maybe it can be saved by saying you must use one or the other but not both; 
then it's "overloaded", but nothing really surprising happens. This way, None 
could be used to say "No ordering -- there should be only one", which is even 
more intuitive. 

We get (semantically):

qset.first('last_name') ==> 
qset.order_by('last_name')[0] if qset.exists() else None

qset.first(None) ==>
qset.get() if qset,exists() else None

qset.first(last_name__startswith='b') ==>
qset.filter(last_name__startswith='b').first(None)

qset.first("last_name", last_name__startswith='b') ==>
raise TypeError("first() takes either all positional args or all keywords")

qset.first() ==>
qset.first(qset.model.ordering)

Note that with this suggestion:

qset.filter(a=b).first(c)

is not equivalent to

qset.order_by(c).filter(a=b)

Because the latter checks for multiple objects and the former doesn't; this. 
IMO, justifies raising a type-error when trying to use both.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: first() and last(), earliest() and latest()

2013-02-27 Thread Shai Berger
Oopsie:

On Thursday 28 February 2013, Shai Berger wrote:
> 
> Note that with this suggestion:
> 
>   qset.filter(a=b).first(c)
> 
> is not equivalent to
> 
>   qset.order_by(c).filter(a=b)
> 
That should have read

qset.order_by(c).first(a=b)

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-02 Thread Shai Berger
On Friday 01 March 2013, Aymeric Augustin wrote:
> Hello,
> 
> I'd like to improve transactions handling in Django. The first step is [to
> replace -- assumed, SB] the current emulation of autocommit with
> database-level autocommit.
> 

There is an issue you seem to be ignoring: An "ORM Write" is, in many cases, 
more than a single query against the backend. The most obvious example is 
models with inheritance -- trying to do these with database-level autocommit 
means that the code writes the two parts in separate transactions. I'm very -1 
on this.

There are two alternatives I see -- one is to make sure that an ORM Write is 
still a transaction (not database-level autocommit); the other is to 
explicitly commit-unless-managed (or something equivalent) after every read 
and raw-sql, as well as write.

BTW, how do you treat select-for-update, which currently makes sense in "faked 
autocommit" mode, but will stop making sense with the suggested change?

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-02 Thread Shai Berger
Thinking again, a few more points come to mind:

On Friday 01 March 2013, Aymeric Augustin wrote:
>
> Such transactions are useless and don't come for free. Relying on them to
> enforce integrity is extremely fragile — what if an external library
> starts writing to a log table in the middle of one of these implicit
> transactions? 

Then it's the external library that is at fault; it should be using a separate 
alias, with its own transactions.

> The term "footgun" comes to mind.
> 
True; but has a wider implication than you seem to give it credit for. In 
particular,

> 
> ** Backwards-compatibility **
> 
> Roughly, I'd classify Django users in four groups:
> 1 - "Transactions, how do they work?"
> 2 - "Django autocommits, doesn't it?"
> 3 - "I'm using TransactionMiddleware!"
> 4 - "I'm managing my transactions."
> 
> Groups 1 and 2 won't see the difference.

This will turn existing projects from correct (though perhaps fragile) to 
buggy, exactly in groups 1&2, where the users are less likely to understand 
the issues. The apps are correct today -- they rely on the documented 
transaction behavior, even if the authors are not fully aware of it. And they 
will turn buggy in a way that is not likely to be detected by tests, because 
it will have effects mostly under concurrent access or system crashes -- 
circumstances you don't usually have in tests.

Thus,
> 
> I don't see much point in providing an option to turn autocommit off,
> because starting a transaction is a much more explicit way to achieve the
> same effect. We usually don't provide "backwards compatibility with bugs".
> 
-1. Make it easier (and cross-backend) to set db-level-autocommit on. Put the 
setting for it in the default template for new projects. Don't change existing 
code from "fragile" to "subtly broken".

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-02 Thread Shai Berger
On Saturday 02 March 2013, Aymeric Augustin wrote:
> On 2 mars 2013, at 16:18, Shai Berger  wrote:
> > -1. Make it easier (and cross-backend) to set db-level-autocommit on. Put
> > the setting for it in the default template for new projects. Don't
> > change existing code from "fragile" to "subtly broken".
> 
> This isn't simply about changing some default. It's about re-implementing
> all the transaction machinery.
> 
> With the scope I described, I estimate that I can complete the project in
> 120 to 200 hours. If I have to keep bug-parity with the current behavior,
> I estimate that I'm not capable of delivering it.
> 
> In other words, you're requesting that I fork Django rather than contribute
> this to the main tree.

I may have phrased my objection too strongly -- all in all, I laud your 
continuing effort to improve Django and its database layer, including this 
change. However, I must stand my ground.

The Django documentation on transactions, at the moment says this on Django's 
default behavior[0]:

> Django’s default behavior is to run with an open transaction which it
> commits automatically when any built-in, data-altering model function is
> called. For example, if you call model.save() or model.delete(), the
> change will be committed immediately.

Now, you want to call this behavior (the part where there's one open 
transaction, rather than one-per-db-operation) a bug. I find that a little odd, 
but I can live with it.

What I have a harder time living with is my other point, which has not been 
refuted: The proposed change turns code that is currently correct -- liable to 
break if changed, but still correct as it stands -- into code with "phantom 
bugs" that show up only in production, under specific timings of concurrent 
requests (because reads and writes which used to be in the same transaction 
are now on separate transactions). It is my opinion that such changes may only 
be made when changing major versions (i.e. Django 2.0), if at all. 

If anyone can offer a way to detect the situation and warn users of the change, 
that would make things perfectly fine. Changing the default transaction 
behavior along the lines you suggested would be a great improvement. I have a 
strong suspicion, though, that if the new behavior is implemented as default 
(and with no recourse to the old behavior, to boot), then that simply cannot 
be done.

I think that as suggested, the change would break the project's committments 
to its users in a way that is much more important than the performance gains, 
avoided idle-in-transaction errors, and effort considerations, all put 
together. That is my objection.

Thanks for your attention,

Shai.

[0] https://docs.djangoproject.com/en/dev/topics/db/transactions/

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-02 Thread Shai Berger
Hi again,

On Sunday 03 March 2013, Aymeric Augustin wrote:
> On 2 mars 2013, at 21:46, Shai Berger  wrote:
> > The Django documentation on transactions, at the moment says this on
> > Django's
> > 
> > default behavior[0]:
> >> Django’s default behavior is to run with an open transaction which it
> >> commits automatically when any built-in, data-altering model function is
> >> called. For example, if you call model.save() or model.delete(), the
> >> change will be committed immediately.
> 
> Yes, my proposal is a backwards-incompatible change with regard to this
> sentence.
> 
> However, I believe that only a small fraction of the users of Django fully
> grok the implications of this sentence, and a fraction of this fraction
> relies on it to ensure some level of integrity.
> 

I agree with this belief about users *deliberately* relying on the 
implications for integrity. I have no idea how many people simply wrote code 
that works, and will now break. I also have no idea how many people maintain a 
system started long ago by a more knowledgeable developer.

> > What I have a harder time living with is my other point, which has not
> > been refuted: The proposed change turns code that is currently correct
> > -- liable to break if changed, but still correct as it stands -- into
> > code with "phantom bugs" that show up only in production, under specific
> > timings of concurrent requests (because reads and writes which used to
> > be in the same transaction are now on separate transactions).
> [...]
> 
> I believe that the level of backwards-incompatibility described above is
> within acceptable bounds for Django 1.6.
> 

I believe this is the core of our disagreement here.

> I also believe that it beats the alternative — namely, live with the
> current behavior forever.
> 

I sincerely hope that is not the only alternative; that there's a way to 
implement the new behavior side-by-side with the old one. There usually is.

> > If anyone can offer a way to detect the situation and warn users of the
> > change, that would make things perfectly fine. Changing the default
> > transaction behavior along the lines you suggested would be a great
> > improvement. I have a strong suspicion, though, that if the new behavior
> > is implemented as default (and with no recourse to the old behavior, to
> > boot), then that simply cannot be done.
> > 
> > I think that as suggested, the change would break the project's
> > committments to its users in a way that is much more important than the
> > performance gains, avoided idle-in-transaction errors, and effort
> > considerations, all put together. That is my objection.
> 
> Yes, I agree with this way to frame the discussion.
> 
> We reach different conclusions because I don't consider backwards
> compatibility an absolute that trumps every other consideration. It's a
> continuum. Backwards compatibility must find a balance with progress.

I don't consider backwards-compatibility an absolute, and the first of my two 
paras which you quoted above shows just that. I do think there are proper ways 
to introduce backwards-incompatible changes, and what you suggested isn't one 
of them.

In particular, per https://docs.djangoproject.com/en/dev/misc/api-stability/, 
we should have a deprecation process with warnings, unless fixing a security 
bug. Even with security issues, it was deemed preferrable to take an expedited 
deprecation process when possible (e.g. with the markdown module) rather than 
an immediate breaking change. Throwing this process to the wind over an 
improvement -- as nice as it is -- seems unwise to me.

An immeiately-breaking change, where the breakage is hard to detect and debug, 
and with high potential for data loss... I don't think any warning in the 
release notes can be enough. All I can say is, -1.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-03 Thread Shai Berger
Hi,

Here's some thoughts on possible problems with Aymeric's plan. I like
the look of the suggested transaction API, I'm raising these to help
make sure it can be realized.

On Sunday 03 March 2013, Aymeric Augustin wrote:
> On 1 mars 2013, at 13:48, Aymeric Augustin 
>  wrote:
> 
> I'd like to add an @atomic decorator that:
>   - Really guarantees atomicity, by raising an exception if you attempt 
> commit within an atomic block,

Fragility alert: databases commit implicitly. Most notably, Oracle and MySql 
for every DDL command.

It is true that the commands that trigger this are not usually found in 
normally-executed application
code. However, since the proposal is all about running in autocommit mode 
unless a transaction
was started explicitly, it should be noted that once such a command is 
executed, it does not
just break the transaction in the middle, it renders everything after it 
non-transactional.

Most DDL commands executed by Django apps, 1.6 and on, should be done through 
the upcoming
schema alteration mechanisms -- code under core's control, so things can be set 
up correctly 
(that is, when such a command is executed under @atomic with a 
non-DDL-transactional backend,
it should raise an exception). They just need care. Other than that, strong 
documentation warnings
should be given around the use of raw sql in transactions.

>   - Supports nesting within an existing transaction, with savepoints.

The use of savepoints in Django apps so far has been very little, as far as I 
know. One point
I'm unsure of is the interaction of savepoints with cursors, since querysets 
are lazy; so the scenario
I'm worrirf about is, generally speaking,

@atomic
def main():
for obj in query_with_more_than_100_objects:
try:
handle(obj)
except Bad:
pass

@atomic
def handle(obj):
if good(obj):
mark_good(obj)
obj.save()
else:
raise Bad(obj)

Will the next (database) fetch after an exception is raised get the right 
objects?

My reading of the Postgresql documentation is that it will do the right thing, 
not so sure about
the other backends.

> 
> I'm reasonably confident that this plan can work.

My concerns about autocommit notwithstanding, I hope you're right.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-03 Thread Shai Berger
On Sunday 03 March 2013, Jacob Kaplan-Moss wrote:
> Shai: do you have any real-world code that'd have data loss
> with this bug?

The code in sites I work on usually uses transaction middleware or 
@commit_on_success, so it would not suffer (nor benefit) from this change. So I 
tried to look for openly available code.

I found this very clear example on Stack Overflow -- I'm not sure it counts as 
"real-world code", but it definitely becomes subtly broken with database-level  
autocommit:

http://stackoverflow.com/questions/8965391/delete-duplicate-rows-in-django-db

Then I turned to somewhere I thought might be susceptible, and took a look at 
django-mptt. I wish I had more time and stamina at this point to find an actual 
smoking gun, but what I did find, was no use of transaction control: Not in 
code (the only transactional code is a call to 
transaction.commit_unless_managed at the end of the move_node operation), and 
not even in documentation (except for docstrings for context-managers which 
delay updates). The code there is pretty complex and abstract, but I'm pretty 
sure it does make decisions based on reads that affect consequent writes -- and 
I'm pretty sure that adding a commit between the former and the latter will 
make it fragile in new and interesting ways. So that is real-world code that 
I'm almost sure now has data loss possibilities, unless used in explicit 
transactions.

Which brings me to this comment:

> Looking through the tons of code I've got available
> can't reveal a single place where this chance would have any negative
> effect. OTOH, there're lots of places where it'd have a positive
> effect.
> 
Please correct me if I'm misunderstanding: For this to hold, you need lots of 
pieces of code where autocommit is on (otherwise there's no positive effect), 
and then you need to be sure that, in all these places, either reads don't 
really affect consequent writes, or some constraint holds that is equivalent to 
serializability -- otherwise, negative effect is possible. That sounds quite 
implausible.

Is the implausible really the case? Otherwise, what am I missing?

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-04 Thread Shai Berger
Hi,

On Monday 04 March 2013, Aymeric Augustin wrote:
> On 4 mars 2013, at 04:04, Shai Berger  wrote:
> > you need to be sure that, in all these places, either reads don't
> > really affect consequent writes, or some constraint holds that is
> > equivalent to serializability -- otherwise, negative effect is possible.
> 
> PostgreSQL and Oracle use the "repeatable read" isolation level by default.
> They are immune to this problem, because a sequence of reads followed by a
> write has the same semantics with a transaction under "read committed" and
> without a transaction.
> 
> MySQL uses "read committed" and SQLite uses "serializable". Users of these
> databases may see a different behavior.
> 
> I'm going to add this to the list of backwards incompatibilities:
> https://github.com/aaugustin/django/commit/876fddb68dd6990e87b15e683c498e41
> f8921f14#L4R437 ("Using unsupported database features" isn't correct for
> MySQL and SQLite right now.)
> 

I want to point out how ironic this whole issue turns out.

First of all, it appears the bulk of users who may experience the subtle 
breakage discussed are users of MySql -- the database of choice for Aymeric's 
groups 1 & 2, the main declared target audience for the fix.

On the other hand, it probably serves them right -- the choice of MySql is 
usually motivated by putting less focus on database-level integrity and more 
on other factors, such as single-threaded performance and ease of deployment.

But considering this last disparaging comment, it's only MySql code that will 
be turned from correct to broken, because -- on this specific point -- it turns 
out MySql is, by default, stricter than the others about integrity.

I am speechless,

Shai.


-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-04 Thread Shai Berger
Hi Florian and everybody,

On Sunday 03 March 2013, Florian Apolloner wrote:
> On Sunday, March 3, 2013 12:27:47 AM UTC+1, Shai Berger wrote:
> > > I also believe that it beats the alternative — namely, live with the
> > > 
> > > current behavior forever.
> > 
> > I sincerely hope [...] that there's a way to implement the new behavior
> > side-by-side with the old one. There usually is.
> 
> Can you describe how an alternative in this case would look like? I
> personally can't think of any sensible one.
> 

This is just a half-baked idea; I suspect those of you who are more well-
versed in the current implementation of transaction management, and who put 
some thought into this, may be able to shoot it down easily. But here goes.

Basically, transaction management reacts to user commands by doing either or 
both of two things: Issuing database commands ("begin tran", "savepoint x", 
"rollbak to x" etc), and changing its own internal state (e.g. marking the 
transaction as dirty, puhsing a new transactional level on a stack). There is 
a limited set of relevant user command classes -- explicit-enter-transaction, 
ORM-read, begin-ORM-write, end-ORM-write, etc.

The selection of state-changes and transaction-commands that are executed in 
response to user-commands is, in essence, a transaction-management policy -- 
and there can be more than one of those in the code base, to be selected by a 
setting. Say, transaction_bc (for "Backwards Compatible") which does 
autocommit the way it is today, and transaction_ad (for "Autocommit at 
Database level") which does the right thing.

If this can be done, then I think the proper way to handle the change is:

Django 1.6 -- the default is transaction_bc, the default project template 
selects transaction_ad. Further, transaction_bc raises a deprecation warning 
whenever it sees an ORM-read followed by an ORM-write in an automatic 
transaction when it knows the transaction isolation level is >= repeatable 
read.

Django 1.7 -- the default is transaction_ad, transaction_bc still available 
but raises deprecation warning whenever it is used (or at least whenever its 
autocommit is used).

Django 1.8 -- transaction_bc removed.

Hope this makes sense,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-05 Thread Shai Berger
On Tuesday 05 March 2013, Aymeric Augustin wrote:
> On 5 mars 2013, at 01:50, Shai Berger  wrote:
> > I want to point out how ironic this whole issue turns out.
> 
> That's a storm in a teapot. Let's keep some perspective:

I really had no intentions of disrupting the weather; however,

> - People using MyISAM aren't affected.

That's been recommended against for ages; InnoDB has been the default since 
MySQL 5.5, 12/2010. Django 1.3 was released three months later, and I think 
you can safely assume the usage of MyISAM in projects developed since 1.3 is 
negligible.

> - People using the transaction middleware aren't affected.

That's your group 3...

> - People using commit_on_success aren't affected.

... and that's your group 4.
> 
> In addition to the above, to end up with data corruption, you must:
> 
> - be aware that MySQL runs on "repeatable read" by default,
> - have checked that Django doesn't set another isolation level — this isn't
> documented,
> - be aware of Django's default transactional behavior —
> virtually no one understands the first paragraph of the transaction docs,
> - have chosen to rely on this behavior to implicitly guarantee some
> integrity between a read and a subsequent write,

No you don't; that's a key point that, for some reason, I haven't been able to 
push through. Code correctness is not dependent upon the state of mind of the 
author.

At least the "delete duplicates" example I've pointed to shows, that it's 
quite easy to write code that is correct now, under the defaults, with MySql 
and will break with the change. It is easy to do so "unwittingly", with no 
real awareness of the details involved (arguably, making it easy to write 
correct code "unwittingly" is one of the goals of frameworks like Django in 
the first place). 

That is exactly the situation that I think is likely for your groups 1&2. I 
don't think the fact that their code is correct "by chance" gives us a license 
to break it.

> - upgrade to Django 1.6 without taking into account the release notes or
> without understanding their consequences for you,

which is well within the norm for these users,

> - be running a medium- or high-traffic website where race conditions are
> likely,

No, that's what you need to end up with large-scale data corruption. I'd 
phrase that point in reverse: To be safe, you need to run a very-low traffic 
website, where concurrency virtually never happens.

> - have code that fails silently rather than raise an error on integrity
> violations.
> 
The problems caused do not necessarily create integrity violations. In the 
deletion example, they just make data go poof.

> That's technically possible, but sufficiently unlikely to be acceptable as
> a _documented_ backwards incompatibility.

I agree that this conclusion follows from your premises. It's those premises 
that I disagree with.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-05 Thread Shai Berger
On Tuesday 05 March 2013, Aymeric Augustin wrote:
> On 5 mars 2013, at 12:50, Shai Berger  wrote:
> > At least the "delete duplicates" example I've pointed to shows, that it's
> > quite easy to write code that is correct now, under the defaults, with
> > MySql and will break with the change.
> 
> http://stackoverflow.com/questions/8965391/delete-duplicate-rows-in-django-
> db
> 
> In fact this code is already unsafe under MySQL with "repeatable read";
> you need "serializable" to make it safe.
> 

I'm not sure what you mean by "unsafe". 

Version 1: code in the question

rows = MyModel.objects.all()
for row in rows:
try:
MyModel.objects.get(photo_id=row.photo_id)
except:
row.delete()

When Django checks for a second record on get() it reads the second record, so 
a repeatable-read transaction would acquire an exclusive read lock on it. This 
makes it impossible for another transaction to delete the second row before 
the first finishes.

Version 2: code in the first answer.

for row in MyModel.objects.all():
if MyModel.objects.filter(photo_id=row.photo_id).count() > 1:
row.delete()

To the best of my understanding, a repeatable-read transaction gets a read 
lock on all records participating in a count, so again, nobody can delete the 
other records before the transaction finishes.

With database-level autocommit, both versions are susceptible to having the 
"other" record deleted between the read that detects it and the deletion of 
the current row (deemed a redundant copy).

> The current behavior doesn't map to any common programming idiom and
> doesn't make much sense in general. The only explanation I've found is "we
> had to commit after making changes otherwise they weren't saved".

There is no argument that the change is wanted. The only issue I'm -1 on is 
changing the default with no deprecation and no option to use existing 
behavior.

> As a consequence, code written "unwittingly" is more likely to be incorrect
> than correct.

Still not a license to break it when it is correct, IMO.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Splicing Geometry by bounding box

2013-03-05 Thread Shai Berger
On Monday 04 March 2013, John Baker wrote:
> Hey I am currently working on a geolocation based game that utilizes a
> geodjango application to manage querying, parsing, and server geographical
> data to our client. We are currently using PostGIS to store our OSM data.
> We are using the OSM tilename system throughout our project and some of our
> geometry spans across many tiles. I would like to reduce specific tile
> payloads by reducing the geometry as much as possible.
> 
> Ideally I would like to "splice" polygons & multipolygons by a bounding
> box, any suggestions?

One meta-suggestion: please take this question to the django-users list. This 
list is for discussing the development of the Django framework itself.

Thanks,
Shai.


-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-05 Thread Shai Berger
On Tuesday 05 March 2013, Aymeric Augustin wrote:
> 
> In practice, this would mean:
> - Define "hook points" that are a superset the old and the new APIs.
> - Re-implement the current transaction management with these "hook points"
>   (hard, because the intended behavior of the dirty flag isn't clear).
> - Implement the new transaction management  with these "hook points"
>   (hard, because that prevents using Python's context manager abstraction,
>   which maps exactly to atomic blocks and guarantees that the exit method
>   is always called).
> - Add checks to prevent invalid operations when mixing the old and new
> APIs, this is extremely difficult to get right when there are many hook
> points. 
> - Provide simple guidelines on how the old and new APIs may be
> mixed. 
> - Ensure that the old API can be trivially removed, because the
> author of the new code may not be there in two years.
> 

I assume "new" and "old" APIs refer to commit_on_success vs. atomic (just 
making sure I got you right)?

> This looks possible, but not in the amount of time I'm able to spend
> volunteering for Django.  If someone wants to implement this, I'll put my
> branch on hold. Otherwise, I'll propose it for inclusion into core.

I couldn't ask for more,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Moving database backends out of the core

2013-03-05 Thread Shai Berger
Hi,

On Tuesday 05 March 2013, Michael Manfre wrote:
> Full disclosure, I maintain django-mssql and am biased toward having all
> database backends treated as if they were 3rd party backends.
> 

In recent years, I have been the main contributor to South's MSSQL and Oracle 
backends. I am biased towards having MSSQL treated as an equal to the database 
systems supported in core, but also towards support of backend-specific low-
level code in user apps. Also, I am a Linux user -- I use django-pyodbc for my 
work on South, not django-mssql.

> 
> Benefits
> ==
>   - One of the biggest benefits of this change would be that database
> backends will no longer be bound to Django's release cycle and Django would
> not need to make a dot release for a database specific issue.
> 
I think you are missing something crucial: Django depends on its commonly-used 
backends. A critical problem with the Postgresql or MySql backend is a 
critical problem for the Django project, regardless of separation to 
repositories. And where the dependency is weaker, being in core was not a 
guarantee for fixes -- Django carried significant Oracle problems for quite 
long, and they were just not deemed crucial.

So, IMO, you can't escape the need to have a set of "blessed" backends, which 
core is committed to their correct functioning, and thus need to be, at least 
to some degree, in sync with the Django main project.

>   - This will result in Django having a more flexible/robust database API.
> Any of the existing complexity imposed on the core by a specific database
> backend could be abstracted out to the API.
> 
>   - In some instances this will simplify the core's code by pushing the
> complexity in to the specific database backend.
> 

These benefits are, IMO, orthogonal to the separation you suggest. I can't 
speak for core, but I doubt that a proposed change to make the database API 
more robust and flexible will be rejected just because it is originally 
motivated by a 3rd-party, rather than core, backend.

> Concerns
> ===
> 
>   - Less popular database backends may get left behind
> 

I am worried that the result of such change will not be that MSSQL is treated 
as well as Postgres is today, but that Oracle is treated as bad as MSSQL is 
today.

> 
> Migration Path:
> ===
> 
> I believe this can be accomplished without needing a multi-version
> deprecation cycle because unlike localflavors, there should be very little,
> if any, user code that imports a specific database backend directly.

South certainly does, I'm sure it's not the only one.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-05 Thread Shai Berger
Hi Ian, Aymeric and all,

I stand corrected.

On Tuesday 05 March 2013, Ian Kelly wrote:
> On Tue, Mar 5, 2013 at 6:38 AM, Shai Berger  wrote:
> > 
> > When Django checks for a second record on get() it reads the second
> > record, so a repeatable-read transaction would acquire an exclusive read
> > lock on it. This makes it impossible for another transaction to delete
> > the second row before the first finishes.
> 

This statement was based in part on my previous understanding of transaction 
isolation levels, and in part on the Wikipedia article about them[1], which 
explicitly names these locks (apparently, following the SQL Stansard).

> SELECT statements without "FOR UPDATE" do not generally acquire
> exclusive locks to my knowledge, even under serializable or repeatable
> read isolation levels.  That would be a major issue for parallel
> requests if they did.  Also, I don't think there's any distinction
> between exclusive read or exclusive write locks; row-level locks are
> either just exclusive or shared.
> 
> I just verified that, at least in PostgreSQL, the logic above works in
> the serializable isolation level but can result in data loss in the
> repeatable read isolation level.

A deeper look in the wikipedia article finds a SIGMOD article[2] which analyzes 
isolation levels and, indeed, shows that such anomalies are allowed by the 
Postgres "Repeatable Read" level (which they name "snapshot isolation").

They also note in passing, that by SQL-92's definitions, this also passes for 
serializable. I note, also, that the stricter (more correct) "serializable" 
definition is new even in Postgres -- only since 9.1[3]; before that, 
Postgresql's "serializable" was really just "snapshot isolation".

> I don't have MySQL handy to test,

I do, and -- as might be expected in view of the above -- it allows the bad 
updates in repeatable-read (the default) as well as serializable isolation 
levels.

So -- I'm giving up on this. As Aymeric noted, on Postgres and Oracle there's 
no harm done, and MySql, as exemplified here, undermines correctness anyways; 
if you are writing your code naively, the change may break your correct code 
in some spot, but it is likely already broken in a dozen others.

One clarification (though I still got it wrong) --

> > Version 2: code in the first answer.
> > 
> > for row in MyModel.objects.all():
> > if MyModel.objects.filter(photo_id=row.photo_id).count() > 1:
> > row.delete()
> > 
> > To the best of my understanding, a repeatable-read transaction gets a
> > read lock on all records participating in a count, so again, nobody can
> > delete the other records before the transaction finishes.
> 
> But with any kind of autocommit on, the transaction ends after the
> row.delete(), unlocking the remaining rows and allowing them to then
> be deleted.

according to [3], in PG/Serializable the count selection would place a 
"predicate lock" on all counted records, preventing wrongful deletes even with 
(current django) autocommit on. But not in repeatable-read, and I don't have a 
PG available to verify.

I'm leaving Ian's test description in the tail of this message for reference.

Sorry for the disruption,

Shai.

[1] http://en.wikipedia.org/wiki/Isolation_(database_systems)
[2] "A Critique of ANSI SQL Isolation Levels", 
http://www.cs.umb.edu/~poneil/iso.pdf
[3] http://www.postgresql.org/docs/9.1/static/transaction-iso.html

--

> and Oracle and sqlite3 support serializable and read committed but not
> repeatable read. I started two transactions in separate psql shells.
> The starting data was:
> 
>  id | photo_id
> +--
>   1 |1
>   2 |1
> 
> The first transaction ran:
> 
> BEGIN;
> SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
> SELECT * FROM TEST WHERE photo_id = 1;
> DELETE FROM TEST WHERE id = 1;
> COMMIT;
> 
> The second transaction ran the same thing but deleted id 2 instead.
> The individual statements were interleaved (so both queries returned
> two rows in the SELECT before either query deleted anything).  The end
> result was that both rows were deleted.  Trying the same thing in the
> serializable isolation level results in this error when attempting to
> commit the second transaction:
> 
> ERROR:  could not serialize access due to read/write dependencies
> among transactions
> DETAIL:  Reason code: Canceled on identification as a pivot, during
> commit attempt.
> 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Switch to database-level autocommit

2013-03-07 Thread Shai Berger
On Wednesday 06 March 2013, Aymeric Augustin wrote:
> 
> So part 3 of the plan is to replace TransactionMiddleware by something
> based on transaction.atomic that actually works. For lack of a better
> idea, I'm considering deprecating the middleware and replacing it with a
> setting. This doesn't qualify as loose coupling; better ideas welcome!

Another half-baked idea: I think atomic_urlpatterns (which wrap the view 
callables in @atomic) could do a lot of the trick. They would leave middleware 
database accesses out of the transaction, which could turn out to be a 
"gotcha", but could be acceptable in many cases.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Test failures under Oracle

2013-03-10 Thread Shai Berger
On Sunday 10 March 2013, Florian Apolloner wrote:
> and provide help there… Eg: https://code.djangoproject.com/ticket/20014 is
> a perfect example where someone with Oracle knowledge can chime in,

Here's a starting point, a query for the constraints affecting a table, from 
the South backend for Oracle: 
https://bitbucket.org/andrewgodwin/south/src/8ecfffbaf5eba680bfe97e61fab6eeaaad53ea2a/south/db/oracle.py?at=default#cl-304

(If nobody else fixes these bugs, I'll definitely get involved, but I can't 
promise when. Thought I could at least share a pointer until then).

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Permissions model limiting the length of verbose-name

2013-03-11 Thread Shai Berger
Hi all,

I would like to draw your attention to bug #8162[1]. It is old as sin, and 
annoying as virtue.

The gist of it is: because, for all models, when the admin is present, 
permissions are created automatically; because these permissions are named 
"Can %s %s" % (action, model.verbose_name); that name gets put in a 50-long 
charfield, and actions "change" and "delete" have length 6 -- so, verbose-name 
cannot have length >39. 

It gets even more annoying with automatic many-to-many fields, which have a 
generated verbose name of "%s-%s relationship", limiting sum of lengths of 
class names to around 25, forcing creation of an explicit "through" model in 
some cases.

The bug was specified as a request to change the Permission model, and as such 
was (and is) triaged as "someday/maybe", supposedly waiting for schema 
alteration in core. Other suggestions to resolve the issue have been ignored; 
in particular, a bug that pointed the problem without suggesting a (wrong, 
IMO) solution was marked as duplicate[2].

Can we re-evaluate this, please?

Thanks,
Shai.

[1] https://code.djangoproject.com/ticket/8162
[2] https://code.djangoproject.com/ticket/17763

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Permissions model limiting the length of verbose-name

2013-03-11 Thread Shai Berger
Hi Russell,

I'm reordering the text I'm replying to below, to make things clearer.

On Monday 11 March 2013 12:14:22 Russell Keith-Magee wrote:
> On Mon, Mar 11, 2013 at 5:58 PM, Shai Berger  wrote:
> > 
> > I would like to draw your attention to bug #8162[1]. It is old as sin,
> > and annoying as virtue.
> 
> You won't get any argument from me on this analysis.
> 
Good :)

> The good news is that it's only the extreme cases that hit this bug --
> class names of with names that are 25 characters or longer -- and if that's
> a description of your codebase, I'd say you've got other problems. 

Sorry, but the good news is wrong; it is not the class name that is inserted 
into the permission name, but the verbose name. The limitation is 39, not 25, 
and yet it is met easily in systems where entities are complex enough. If this 
weren't the case, the bug wouldn't be annoying.

The limitation of ~25 is not for single class names -- it is for combinations 
of two class names, in the specific case of automatic many-to-many fields. 
I hope you'd agree that 12 is too little for class names even in 
Python. This is not Java, but not APL either.

> You say that "Other suggestions to resolve the issue have been ignored" -
> I'm unaware of any such suggestions. Can you point at or summarise them? If
> there's a viable option, I'd be in favour of applying that fix, but I
> genuinely can't say I see any option that is both backwards compatible, and
> doesn't require schema migrations.
> 
The "name" field of permissions is filled this way because it is, primarily, a 
display name. The field that is intended for lookup is codename; a unique 
constraint is only applied to codename.

My suggestion for a fix, therefore, is to change the generation of the field 
value, cutting the generated value down to 50. With most database backends, 
there can be no backwards-compatibility problem -- the "current behavior" 
value couldn't have been saved anywaty. With SQLite and MySql-in-non-strict-
more, the behavior is arguably a bug, but I'd rather special-case them somehow 
and solve the problem for the rest of us.

> > 
> > Can we re-evaluate this, please?
> 
> Sure.
> 
> Has schema alteration been added to core? No.
> 
> Evaluation complete :-)
> 
As I said, I think schema change is the wrong solution here anyway. That is 
what I want re-evaluated.

Thanks,
Shai

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Changes to django's settings module

2013-03-15 Thread Shai Berger
Omer,

To convince the core committers that the patch is valuable, you need to show 
how it improves things. Build examples for the use-cases mentioned in the 
thread, show how they would be done without your patch, and how the presence 
of your patch allows improvements. The main argument against the patch, as far 
as I've seen, is that it doesn't really help with the common use-cases; the 
onus is on you to refute this claim by examples.

(also, using paragraphs will make your mails easier to read).

HTH,
Shai.

On Friday 15 March 2013, Omer Katz wrote:
> By the way,
> The fact that I mentioned that this patch can be extended to load packages
> as well doesn't mean it's the only use case.
> I asked you guys if you think that this is needed.
> It will enable you to split settings as packages per deployment
> (development package, staging package, production package) and split the
> settings themselves by topic through different modules if you want to.
> 
> בתאריך יום שישי, 15 במרץ 2013 11:14:31 UTC+3, מאת Russell Keith-Magee:
> > On Fri, Mar 15, 2013 at 3:36 PM, Omer Katz
> > 
> > 
> > > wrote:
> >> Why would you call them magic?
> >> Why does allowing extensibility for those who need it is a bad idea?
> >> You will be doing it explicitly anyway by providing a SettingsCollector
> >> class to the Settings class' constructor.
> >> If are doing it, you should know what you are doing.
> >> . Is my code harder to debug or understand than the current code? I
> >> strongly disagree here. The current Settings class clearly violates
> >> SRP. It
> >> holds the settings, validates them and collect them. What's not easy to
> >> understand about SettingsCollector? You can understand by it's name
> >> what exactly it does.
> >> 
> >> "The only reason for having these SettingsCollectors in core is to allow
> >> redistributable apps like django-configurations to do magic. That is, so
> >> they can say, "Install django-configurations, and suddenly every setting
> >> can be overridden from the environment!" This seems like a bad idea,
> >> versus "Install django-configurations, and now you have a new tool in
> >> your toolbox to put in settings.py that imports from the environment!""
> >> Why exactly this is a bad idea? Since when extensibility is a bad idea
> >> in software development?
> >> You just say you are against it but you don't provide a good reason
> >> other than it's easier to explain to others what's wrong if something
> >> is wrong.
> >> 
> >>  The current behavior will stay as the default behavior *at least *until
> >> 
> >> Django 2.0 if not forever so "it's easier to explain to others what's
> >> wrong" is not a valid argument in my opinion.
> >> If you are writing your own SettingsCollector you probably know what you
> >> are doing.
> >> If we'll introduce other types of SettingsCollectors in Django then we
> >> won't introduce them in the Getting Started documentation until a very
> >> late stage so that newcomers can understand the default behavior but
> >> that's another issue for later on.
> >> 
> >> For now I want to focus on the pull request itself.
> >> Does the refactoring makes the code clearer in it's intension?
> >> Does it allow extensibility when it is required?
> >> Does it maintain the default behavior for most Django developers?
> >> 
> >> I believe that the answer to all of those questions is yes it does.
> > 
> > I'd definitely argue the first point - that it makes the code clearer.
> > 
> > If you break up your settings file using normal Python imports, the order
> > of evaluation of a settings file is clear, and can be followed by anyone
> > that understands the normal Python import process.
> > 
> > In order to use your patch, you need to understand how settings modules
> > will be combined, and in what order, and with what precedence. It relies
> > upon implicit knowledge of the mode of operation of the collector, rather
> > than the explicit behaviour of a built-in language feature.
> > 
> > As for the second point -- As I've said previously, I'd argue that yes,
> > it allows extensibility, but not on any axis that, in my experience,
> > requires it. I've seen a range of problems related to the structure of
> > settings files, but none of them require the sort of structure that
> > you're making easy to achieve here. If you want to propose a way to make
> > it easy to separate production from development settings, or a way to
> > keep private settings (like passwords and SECRET_KEY) out of
> > repositories, I'm all ears -- but I suspect these problems are more an
> > issue of documenting conventions than of adding features.
> > 
> > The last point is moot - backwards compatibility is definitely required
> > for any feature going into core, but that doesn't mean that just because
> > a feature is backwards compatible it will be added to core.
> > 
> > Yours,
> > Russ Magee %-)

-- 
You received this message becau

Re: Persistent connections, take 2

2013-03-18 Thread Shai Berger
On Monday 18 March 2013 16:36:53 Aymeric Augustin wrote:
> By default, the development server creates a new thread for each request it
> handles. Not only does this negate the effect of persistent connections
> (they're thread-local), 
> [...]

> 1) Do we want to enable persistent connections in production by default?
> 
> 2) Assuming the answer to 1) is yes, can we fix runserver?
>   a) close connections at the end of the dev server request handler
>   => creates tight coupling between the dev server and the ORM :(
>   b) disable persistent connections when DEBUG  = True
>   => badly targeted: some people may use runserver with DEBUG = 
> False,
>   or want persistent connections when DEBUG = True 
>   c) ???
> 

If the persistent connections are thread-local, don't you want to close them 
anyway when the thread exits?

> Florian independently discovered the same problem with gunicorn + gevent,
> because the gevent worker runs each request in its own "thread"
> (greenlet).

... but that fix will kill persistent connections under gunicorn.

The solution that sounds "right" is to pool the persistent connections -- 
every thread that ends returns the connection to the pool.

If I understand correctly, this would tie the ORM not to the dev server, but 
to the wsgi handler -- but in a sense where, actually, it should be tied.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Is there a buildbot? Is there a waterfall? Do the tests pass against all backends?

2013-03-18 Thread Shai Berger
Hi,

Reviving an oldish thread:

On Tuesday 26 February 2013 00:35:10 Florian Apolloner wrote:
> it would be of great help
> if you could setup cx_Oracle on an Ubuntu 11.10 (I have to double check
> that tomorrow, don't have my ssh keys with me currently) 64 bit system with
> python 3.3 and see if you can get the tests running or if you also get a
> segfault.
> 

I got as far as performing a simple query, no segfault. As I'm writing this, 
I'm downloading the Django sources to the Ubuntu 11.10 VM in order to try 
running the tests.

As far as I know, I did nothing special for this -- just compiled Python from 
source, unzipped the instantclient packages, and used them to build cx_Oracle 
from source in a Python3 virtual environment ("pyvenv").

Hope this is still of interest,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Persistent connections, take 2

2013-03-18 Thread Shai Berger
On Monday 18 March 2013, Aymeric Augustin wrote:
> On 18 mars 2013, at 17:10, Shai Berger  wrote:
> > If the persistent connections are thread-local, don't you want to close
> > them anyway when the thread exits?
> 
> Yes, do you know how this could be achieved? I haven't found how to hook
> on thread termination.
> 

You can get a list of the "living" threads from threading.enumerate(), so it 
is possible to find out that a thread has already ended. This probably quite 
inefficient, but may be good enough for runserver.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Is there a buildbot? Is there a waterfall? Do the tests pass against all backends?

2013-03-19 Thread Shai Berger
On Monday 18 March 2013, Shai Berger wrote:
> Hi,
> 
> Reviving an oldish thread:
> 
> On Tuesday 26 February 2013 00:35:10 Florian Apolloner wrote:
> > it would be of great help
> > if you could setup cx_Oracle on an Ubuntu 11.10 (I have to double check
> > that tomorrow, don't have my ssh keys with me currently) 64 bit system
> > with python 3.3 and see if you can get the tests running or if you also
> > get a segfault.
> 
Segfault after ~3850 tests (including several errors and failures, but that's 
minor).

Is there any interest in fixing this, specifically? My guess is that the 
intersection between Oracle users and Python3 users (within Django users) is 
pretty slim, and further, Ubuntu 11.10 (a non-current, non-LTS release) is  a 
prime target for neither Oracle (stable-liking) users nor Python3 (new-
preferring) users.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Is there a buildbot? Is there a waterfall? Do the tests pass against all backends?

2013-03-30 Thread Shai Berger
On Tuesday 19 March 2013, Florian Apolloner wrote:
> Hi,
> 
> On Tuesday, March 19, 2013 8:21:05 AM UTC+1, Shai Berger wrote:
> > Is there any interest in fixing this, specifically?
> 
> Sure, I just don't have to knowledge to debug cx_Oracle, so if you are up
> to please. Although I think the endresult would most likely be a patch to
> cx_Oracle and not Django.
> 
I had meant, is there interest in solving it on Ubuntu 11.10 -- things like 
segfaults are sometimes related to specific versions of libraries and even 
compilers.

But I was able to generate a segfault also on my main work machine, a Debian 
Testing. It even seems much worse there. So it will be a little easier for me 
to work on, and there may be some more motivation for others to work on it.

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: [SPAM] unittest2 discovery proposal

2013-04-01 Thread Shai Berger
Hi,

+1 in general. One concern, and one idea:
>
> -- Backwards-incompatible changes --
> 
> * Some valid test structures in Django don't work with unittest2. For
>   instance, tests in `tests/__init__.py` don't match a patter than
>   unittest would recognize if running discovery on a module.
> 
I have a complex app with many tests; so many, that we felt the need to break 
the tests module into modules in a package. So we have 
tests/test_frobnication.py
tests/test_grobulation.py
etc., and
tests/__init__.py
which imports them all.

It isn't quite clear to me how this applies to such a structure; it seems like 
I would need a special pattern, "tests.test_*.py" or some such. This, in turn, 
raises an idea: an app should be able to specify the discovery parameters for 
its own tests in the source, not just on the command line.

Just to be clear: In view of backwards-compatibility concerns, I don't think 
the load_tests protocol supported by unittest2 is enough for this. It should 
be much simpler, IMO; something like (pseudo code):

try:
from app import tests
if hasattr(tests, 'root'):
set_discovery_root(tests.root)
if hasattr(tests, 'pattern'):
set_discovery_pattern(tests.pattern)
except ImportError:
pass

This way, my case is handled simply by adding these lines into 
tests/__init__.py:

root=app.tests

Thanks,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Changing deferred model attribute behavior

2013-04-26 Thread Shai Berger
On Friday 26 April 2013, Alex Gaynor wrote:
> Sorry, I misunderstood the original request. Yes, you're right Anssi and
> Adrian, finding them on demand is reasonable.
> 
Reasonable, but not entirely necessary for this.

I have code that requires something like that; when it is time to "undefer" 
the object, I just load it up anew from the db. I think this covers Adrian's 
use case well, except (perhaps) for the part where it is triggered by 
attribute access.

(my own use case is going through a list of objects and serializing all of 
them; deferring is necessary because the query that selects the objects needs 
to use distinct(), and some objects have TextFields -- which would make the 
"distinct" so non-performant, that Oracle simply forbids it).

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Changing deferred model attribute behavior

2013-04-28 Thread Shai Berger
Hi again,

On Friday 26 April 2013, Anssi Kääriäinen wrote:
> [...]
> In almost every case it is better to try to minimize the amount of
> queries than the amount of loaded fields. The cases where you have
> deferred multiple fields, need only one of them later on, and there is
> a field that you can't load due to potentially huge value are
> hopefully rare.
> 
> But, I don't really care too much. If the objects come from DB queries
> instead of something like the use case of Adrian then if you end up
> doing deferred field loading you have already failed.

LOBs in Oracle always require at least a separate fetch operation to read 
them. Thus, you get an extra database hit per field (per record). Getting them 
before they are needed is almost always inefficient. As I already noted in this 
thread, they can even get in the way of query execution if not deferred.

I think it would be better if, at least on Oracle, such fields could be 
deferred by default. We are considering achieving this with an introspective 
custom manager, applied to all models (or at least all models with TextFields) 
in our project. If I could do it with a special field type, I would, and I'd 
recommend every other user do the same.

Just a data point for this discussion,

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: design decisions for django-mssql: (switching on autocommit and remote proxy) advice wanted.

2013-04-30 Thread Shai Berger
On Tuesday 30 April 2013, VernonCole wrote:
> Dear knowledgeable persons:
> 
> I have completed a beta-test version of a Linux remote access server for
> adodbapi, and have started the process if integrating them into
> django-mssql.   (This is going to be an interesting experience for me -- I
> have already identified two or three new features that adodbapi needs to
> have.) I would like to get your collective wisdom about how to best
> accomplish two things.
> 
> 1) The transaction logic will change drastically for django 1.6.  For the
> supported backends it makes good sense to simply change that in the
> repository. However, it is reasonable to expect significant code change
> with the upgrade of this backend, and I think that we should maintain a
> single code base for a while.  I was thinking of behavior similar to what I
> do inside of adodbapi:
> 
> ...if sys.version_info >= (3,0):  # note that the "if" is executed at
> import time, not run time.
> ..maxint = sys.maxsize
> ...else:
> ..maxint = sys.maxint
> 
> Is there something more appropriate than django.VERSION to use for this, or
> is it, perhaps, just a bad idea?
> 

I work with a company who uses Django with MSSQL, and traditionally stays 
behind Django releases (e.g. we moved to 1.4 only around the release of 1.5). 
For us to be using this in the next two years, I'd say that this kind of 
backwards-support is a must.

> 2)  In my unit test programs, I detect whether to use the proxy module or
> the regular module depending on whether their is a certain key in the
> connection arguments.
> 
> .. import adodbapi
> ...import adodbapi.remote
> 
> ...if "proxy_host' in connection_keywords:
> ..db = adodbapi.remote
> ...else:
> ..db = adodbapi
> ...db_connection = db.connect(connection_keywords)
> 
> Would there be any technical or political reasons NOT to do that in
> sqlserver_ado/base.py ?
> 

It isn't quite clear to me how this would reflect in user code. If (as I 
suspect) this is just another key in the OPTIONS dictionary inside the 
DATABASES entry, then it's perfectly ok. We have to use different entries for 
Linux and Windows anyway.

HTH,
Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




  1   2   3   4   5   6   7   >