On 7/3/15 1:55 PM, Aymeric Augustin wrote:
2015-07-03 17:10 GMT+02:00 Berker Peksağ <berker.pek...@gmail.com <mailto:berker.pek...@gmail.com>>:

    I agree with you on the StringIO and BytesIO cases, but I agree with
    Andriy on the other usages (e.g. ZipFile, GzipFile). Many contributors
    follow the existing practices to write new tests, so it would be nice
    to use best practices if they don't produce too much noise.


StringIO and BytesIO are different from files and sockets. The former only use RAM while the latter use file descriptors, which are in much scarcer supply
(perhaps 1024 per process).

Leaking file descriptors can cause the process to crash because it exceed the
systems limit, so it's really bad.

Leaking StringIO or BytesIO doesn't change anything. They'll be closed when
the garbage collector releases them.

That's why I don't believe the analogy holds.

Many of the StringIO and BytesIO changes in the pull requests won't change anything. The with-statement extends to the end of the test method, which is when the object would be reclaimed anyway:

   @@ -31,10 +32,10 @@ def write(self, data):
                 self.bytes_sent += len(data)
# XXX check Content-Length and truncate if too many bytes written?
   -        data = BytesIO(data)
   -        for chunk in iter(lambda: data.read(MAX_SOCKET_CHUNK_SIZE), b''):
   -            self._write(chunk)
   -            self._flush()
   +        with contextlib.closing(BytesIO(data)) as data:
   +            for chunk in iter(lambda: data.read(MAX_SOCKET_CHUNK_SIZE), 
b''):
   +                self._write(chunk)
   +                self._flush()
def error_output(self, environ, start_response):
             super(ServerHandler, self).error_output(environ, start_response)


I don't think it makes sense to blindly use contextlib.closing on anything that acts like a file, even when we know it is a harmless StringIO. Better to have developers think about what is happening, and use the appropriate construct.

When I see " contextlib.closing(BytesIO(data))", I think, this person is following a pattern, and doesn't understand.

--Ned.

--
Aymeric.
--
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 <mailto:django-developers+unsubscr...@googlegroups.com>. To post to this group, send email to django-developers@googlegroups.com <mailto:django-developers@googlegroups.com>.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CANE-7mXog8YE8LMSxwokaMddjDjcK949wUNzEd_NnLtcknWE_A%40mail.gmail.com <https://groups.google.com/d/msgid/django-developers/CANE-7mXog8YE8LMSxwokaMddjDjcK949wUNzEd_NnLtcknWE_A%40mail.gmail.com?utm_medium=email&utm_source=footer>.
For more options, visit https://groups.google.com/d/optout.

--
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 post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/5597D718.9090701%40nedbatchelder.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to