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

Calling ReportService through a managed NAT resulting in closed connections #250

Closed
oaustegard opened this issue Feb 7, 2018 · 9 comments
Assignees

Comments

@oaustegard
Copy link

We have attempted to utilize both this and the Java lib to download DFP reports from the ReportService through a managed NAT service, whose default timeout is 350 seconds. The absence of a Keep-Alive header on the http call forces the NAT to drop our connection prematurely.

https://github.com/googleads/googleads-python-lib/blob/master/googleads/common.py#L681 sets the socket timeout of the call to 1 hr, and this allows us to download the report from a public subnet, but our process must run behind the NAT and we're thus stymied with the current code base.

Is it possible to either a) add a Keep-Alive header with a similar timeout as in L681, or b) allow users to pass in the needed headers as part of the call? We'd hate to have to edit and keep synchronized changes to the library.

Thanks!
Oskar

@grivescorbett
Copy link
Contributor

Hey Oskar!

Thanks for writing in, I haven't heard this use case before. Just to be sure I fully understand, you'd like to be able to insert a custom http header in the request to the report download URL that's generated here? Do you have a similar desire to do so with the soap request http headers set here?

Best,
Gabe

@oaustegard
Copy link
Author

oaustegard commented Feb 9, 2018 via email

@oaustegard
Copy link
Author

oaustegard commented Feb 14, 2018

An update on this - while we would still like the library to handle keep-alives (or allowing us to specify it as a header parameter, without the need for us to alter the library code), one of our intrepid platform engineers came up with a workaround:

import httplib

orig_connect = httplib.HTTPConnection.connect
def monkey_connect(self):
    orig_connect(self)
    self.sock.setsockopt(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1)
    self.sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_KEEPIDLE, 30)
    self.sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_KEEPINTVL, 10)
    self.sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_KEEPCNT, 6)
httplib.HTTPConnection.connect = monkey_connect

Basically we're keeping the NAT Gateway open by monkey-patching the httplib.HTTPConnection.connect function with the appropriate header applied to the socket directly. A hack, but appears to work.

@grivescorbett
Copy link
Contributor

Hey Oskar,

Apologies for the delay, but I'm glad you're unblocked. This doesn't look the same as setting an HTTP header to me though, is this really what you were after? Exposing the raw socket is pretty different to exposing support for custom headers.

@oaustegard
Copy link
Author

oaustegard commented Feb 15, 2018 via email

@grivescorbett
Copy link
Contributor

Hey Oskar,

I hear you, I did not know that you could specify socket flags like that using HTTP headers. Again I'm happy that you're unblocked, and we'll explore building this properly into the library.

@grivescorbett
Copy link
Contributor

Hi Oskar,

Quick update: we've agreed on an API and I've begun working on this feature.

Cheers!
Gabe

@oaustegard
Copy link
Author

Excellent! I look forward to being able to strip our hack and replace with the formal library upgrade!

@msaniscalchi
Copy link
Contributor

This was fixed in the last release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants