Skip to content

Conversation

@fumin
Copy link
Contributor

@fumin fumin commented Aug 7, 2014

One such use case is to use the library within the constraints of Google
App Engine
https://developers.google.com/appengine/docs/go/sockets/reference

One such use case is to use the library within the constraints of Google
App Engine
https://developers.google.com/appengine/docs/go/sockets/reference
conn.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

This line doesn't appear to work in Go 1.0.

@johto
Copy link
Contributor

johto commented Aug 9, 2014

I've added some comments about the implementation, but in general I consider the idea workable.

@fumin
Copy link
Contributor Author

fumin commented Aug 17, 2014

Thanks for the comments, the part on the DialTimeout case is especially important. I have also ensured backwards compatibility down to go1.0.2 .

@johto
Copy link
Contributor

johto commented Sep 16, 2014

My apologies, @fumin, I wasn't aware you had updated this PR. On a quick look it looks like you've addressed all my comments, so I'll have another look this week. Sorry for the delay.

@fumin
Copy link
Contributor Author

fumin commented Sep 16, 2014

@johto no worries, I have resolved the conflicts due to changes in the master repo in the meantime. However, I see that the build failed for the combination of go1.2 and pg9.3 with a connection timeout error. I ran this combination a couple of times locally and every time all tests passed. Could the failure be due to just a temporary glitch in the build cluster?

@johto
Copy link
Contributor

johto commented Sep 18, 2014

However, I see that the build failed for the combination of go1.2 and pg9.3 with a connection timeout error. I ran this combination a couple of times locally and every time all tests passed. Could the failure be due to just a temporary glitch in the build cluster?

I suppose that's possible. But it is a bit suspicious that after changing how dialing works connecting suddenly times out.. I don't see anything obvious in the code, though, but I'll have a closer look later.

In the meanwhile, is there a reason to require a pointer receiver in defaultDialer? It seems completely unnecessary.

@fumin
Copy link
Contributor Author

fumin commented Sep 19, 2014

Right, it might be clearer for defaultDialer to be a struct instead of a pointer, I've updated this change. However, this time the build failed on a different combination go1.3 + pg8.4 on a different test case TestListenerFailedQuery. I ran this combination on my computer and scrutinized the code a few more times but couldn't find anything decisive except noticing that errRecover has been replaced. Not being able to reproduce the error on my machine makes it hard for me to investigate, I wonder are there other ways to reproduce the problem?

@johto
Copy link
Contributor

johto commented Sep 23, 2014

Right, it might be clearer for defaultDialer to be a struct instead of a pointer, I've updated this change. However, this time the build failed on a different combination go1.3 + pg8.4 on a different test case TestListenerFailedQuery. I ran this combination on my computer and scrutinized the code a few more times but couldn't find anything decisive except noticing that errRecover has been replaced. Not being able to reproduce the error on my machine makes it hard for me to investigate, I wonder are there other ways to reproduce the problem?

I can't see anything wrong with the code, nor can I reproduce the problem in any of the environments. I've committed this in 5ff334f.

Thanks!

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.

2 participants