use urlopen context manager to handle connection close in a timely manner

[jubrad]

What it does:

More explicitly closes the xapi response object to avoid downstream circular reference or other potential issues where garbage collector does not promptly clean the request and close the connection.

Background:

I'm noticing many "Session Timed Out" errors while using local-execs in terraform to configure some resources that the tf provider cannot. At first I tested to see what rate I would hit this bug, but was not able to with >6k requests happening serially (creating and deleting the same dns proxy static entries). Since terraform runs with 10 parallel threads walking its tree I decided to test some parallel calls and quickly was able to rerpro. ~20-40 threads making 4 entries each.

I believe this relates to terraform-provider-panos/issues/255, at least it does for me, but it's possible there's something wonky going on with the PA server here.

How it was tested:

Using a python script that adds and removes dns-proxy static entries I validated that with ~20 processes each making 8 entries as fast as they can the python client would regularly produce "Session Timed Out" errors. My goal of this test was to see if I could increase the rate of collision or push the server beyond its concurrent open request limits. These "errors" would produce a valid http response to the urlopen response object with the following error:
b'<response status="unauth" code="22"><msg><line>Session timed out</line></msg></response>'
To me, this indicates that there's something weird going on on the server side, potentially to do with auth, but probably not due to any tcp / request issues.

When using the context manager I could not reproduce this error in python, even when increasing the concurrent processes to 40. Although... the terraform provider does give me a session timed out so there's still more for me to figure out.

Not the best solution, but it's what I have time for.

[devbollinger]

Hello guys, we do have the same error and looking for a way to at least close the connection manually.
The code in the commit looks very interesting and would probably help.
Is it going to be merged?

[kevinsteves]

This fix looks good, resulting in response.closed is True when the context manager block is exited. Is there a reason we can't simplify the urlopen() part to be just:

            with urlopen(**kwargs) as response:
                response.body = response.read()

[devbollinger]

Seems good to me

[kevinsteves]

Modified version of
#47
from Justin Bradfield merged.

[jubrad]

This fix looks good, resulting in response.closed is True when the context manager block is exited. Is there a reason we can't simplify the urlopen() part to be just:

            with urlopen(**kwargs) as response:
                response.body = response.read()

@kevinsteves I think I did this to log a few things during debug. I then removed the log messages and didn't put the code back to the simpler form. Thanks for merging.

[jubrad]

Hello guys, we do have the same error and looking for a way to at least close the connection manually. The code in the commit looks very interesting and would probably help. Is it going to be merged?

@devbollinger make sure that you're not exceeding the max threads of the PA server... There was a longer discussion in the pango repo about that. This is especially applicable in the terraform provider, if have parallelization > 4 then you will hit timeout issues to some extent.