Add BaseEventLoop._closed attribute

Add BaseEventLoop._closed attribute

[GvR, 2014-05-28 23:05:27]

https://codereview.appspot.com/98680044/diff/1/asyncio/base_events.py
File asyncio/base_events.py (right):

https://codereview.appspot.com/98680044/diff/1/asyncio/base_events.py#newcode135
asyncio/base_events.py:135: self._closed, self.get_debug()))
I wonder if we should also add a property or accessor method so a user can test
the closed flag. There are a lot of places in the patch now that use this
private variable.

https://codereview.appspot.com/98680044/diff/1/asyncio/proactor_events.py
File asyncio/proactor_events.py (right):

https://codereview.appspot.com/98680044/diff/1/asyncio/proactor_events.py#new...
asyncio/proactor_events.py:356: if self._closed:
I'm not sure if this is an improvement. Now you are depending on an
implementation detail of the superclass.

https://codereview.appspot.com/98680044/diff/1/asyncio/selector_events.py
File asyncio/selector_events.py (right):

https://codereview.appspot.com/98680044/diff/1/asyncio/selector_events.py#new...
asyncio/selector_events.py:58: if self._closed:
Ditto.

https://codereview.appspot.com/98680044/diff/1/tests/test_selector_events.py
File tests/test_selector_events.py (left):

https://codereview.appspot.com/98680044/diff/1/tests/test_selector_events.py#...
tests/test_selector_events.py:95: def test_close_no_selector(self):
Did you mean to delete this test?

https://codereview.appspot.com/98680044/diff/1/tests/test_selector_events.py
File tests/test_selector_events.py (right):

https://codereview.appspot.com/98680044/diff/1/tests/test_selector_events.py#...
tests/test_selector_events.py:81: 
You don't need this blank line.

[yselivanov, 2014-05-28 23:07:09]

https://codereview.appspot.com/98680044/diff/1/asyncio/base_events.py
File asyncio/base_events.py (right):

https://codereview.appspot.com/98680044/diff/1/asyncio/base_events.py#newcode133
asyncio/base_events.py:133: return ('<%s running=%s closed=%s debug=%s>'
Hm, maybe if event loop is closed, then we only need <EventLoop closed> form,
and when it's not closed, we can shod <Event loop running=True debug=False>
form?

https://codereview.appspot.com/98680044/diff/1/tests/test_selector_events.py
File tests/test_selector_events.py (left):

https://codereview.appspot.com/98680044/diff/1/tests/test_selector_events.py#...
tests/test_selector_events.py:103: self.assertIsNone(self.loop._selector)
Are you sure it's a good idea to remove these tests?

[haypo_gmail, 2014-05-28 23:18:45]

https://codereview.appspot.com/98680044/diff/1/asyncio/base_events.py
File asyncio/base_events.py (right):

https://codereview.appspot.com/98680044/diff/1/asyncio/base_events.py#newcode135
asyncio/base_events.py:135: self._closed, self.get_debug()))
On 2014/05/28 23:05:26, GvR wrote:
> I wonder if we should also add a property or accessor method so a user can
test
> the closed flag. There are a lot of places in the patch now that use this
> private variable.

File objects of the io module have a read-only closed property. We can add a
get_closed() method to be consistent with other flags: get_debug() for _debug.

It is useful to know if the event loop is closed or not.

https://codereview.appspot.com/98680044/diff/1/tests/test_selector_events.py
File tests/test_selector_events.py (left):

https://codereview.appspot.com/98680044/diff/1/tests/test_selector_events.py#...
tests/test_selector_events.py:103: self.assertIsNone(self.loop._selector)
On 2014/05/28 23:07:09, yselivanov wrote:
> Are you sure it's a good idea to remove these tests?

These checks should be moved to test_close().

[GvR, 2014-05-28 23:45:03]

Since event loops don't have any other properties I think get_closed() is the
thing to do.

[yselivanov, 2014-05-28 23:50:18]

On 2014/05/28 23:45:03, GvR wrote:
> Since event loops don't have any other properties I think get_closed() is the
> thing to do.

Maybe is_closed(), similar to is_running()?

[GvR, 2014-05-28 23:51:47]

Oh, fine.


On Wed, May 28, 2014 at 4:50 PM, <yselivanov@gmail.com> wrote:

> On 2014/05/28 23:45:03, GvR wrote:
>
>> Since event loops don't have any other properties I think get_closed()
>>
> is the
>
>> thing to do.
>>
>
> Maybe is_closed(), similar to is_running()?
>
> https://codereview.appspot.com/98680044/
>



-- 
--Guido van Rossum (python.org/~guido)

[GvR, 2014-06-03 03:11:34]

LGTM. You may shorten the __repr__ if not closed (as Yuri suggested), or you may
leave it as is.

[haypo_gmail, 2014-06-03 23:19:33]

On 2014/06/03 03:11:34, GvR wrote:
> LGTM. You may shorten the __repr__ if not closed (as Yuri suggested), or you
may
> leave it as is.

I disagree with Yury's suggestion for __repr__ because it is possible to close
an event loop while it is running (running=True). But it looks like a bug in
asyncio:
https://code.google.com/p/tulip/issues/detail?id=171

I will reconsider Yury's suggestion when this issue will be solved.