Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dont remove reference to _UnixReadPipeTransport #376

Merged
merged 1 commit into from
Feb 7, 2017

Conversation

gescheit
Copy link
Contributor

@gescheit gescheit commented Sep 7, 2016

_UnixReadPipeTransport class closes pipe in del method. This commit
adds attribute to SpawnBase with pointer to current transport.

_UnixReadPipeTransport class closes pipe in __del__ method. This commit
adds attribute to SpawnBase with pointer to current transport.
class PatternWaiter(asyncio.Protocol):
transport = None
def __init__(self, expecter):

def set_expecter(self, expecter):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like this pattern of having a non-constructor method which must be called before other methods will work.

I've probably used asyncio wrong in some way when writing this code, but I don't really have the time or interest to work out the right way to do it at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that you can apply this workaround with "no-like" pattern and resolve critical bug. When you got more free time you can make more accurate resolution of this problem. At this time expect() with "async" option totally unusable. Also need to rename async because in python 3.5 async is name of operator.

Copy link
Contributor

@MountainRider MountainRider Sep 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't the expecter be passed to the constructor? Was the constructor removed so that the super class constructor would be called instead? I don't understand this change. Ok. I looked at the usage some more. I see that if the async_pw_transport is already set, the set_expecter method is used to set the expecter. Why can't the constructor call set_expecter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because instance of PatternWaiter should not be destroyed. If this happens socket will be closed. So i save PatternWaiter instance in SpawnBase instance.

@mbrar
Copy link

mbrar commented Feb 6, 2017

Hi there, I would really like to see this code merged in. Thanks!

@takluyver
Copy link
Member

OK, there's little point in holding out for a better solution if I'm not prepared to work on one.

@takluyver takluyver merged commit cde69e4 into pexpect:master Feb 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants