Fix sse and nginx buffering

[guzzijones]

Environment:

PYTHON_VERSION=python3.6 make requirements.txt

setup nginx st2.conf listening on 8443

server {
  listen       *:8443 ssl;
  client_max_body_size 5M;

  ssl on;

  ssl_certificate           /etc/ssl/st2/st2.crt;
  ssl_certificate_key       /etc/ssl/st2/st2.key;
  ssl_session_cache         shared:SSL:10m;
  ssl_session_timeout       5m;

client call:

python -m st2client.shell --debug  --auth-url https://localhost:8443/auth --api-url https://localhost:8443/api --stream-url https://localhost:8443/stream --cacert=False --skip-config pack install /home/ubuntu/utilstack --python3

What is the problem you're trying to solve with this patch?

#4842

What's the solution?

Solution is to watch the execution id status events and look for one of the LIVEACTION_STATUS_COMPLETED events

What's end event and end execution ID do?

tell the generator function in stream to stop sending events back to the client

How is the cacert option has to do with this change?

allows stream to use the --cacert option via command line. Needed this to test the change using a self signed certificate.

Proxy timeout?

sets a reasonable timeout for connection and for read. other endpoints have it as well.

[guzzijones]

Well it turns out setting the timeout will cut off the stream going back to the client which causes other issues client side. Going to have to keep digging here.

[guzzijones]

looks like circle and travis had issues.
afaik this PR should pass.
Leaving it here for review.

[guzzijones]

Looks like this section of code is missing unit testing. I will create a few.

[m4dcoder]

There's no description for the PR. It looks like you're trying to fix a hang on your setup. You have to provide more information in the description. How is your environment setup? What is the problem you're trying to solve with this patch? What's the solution? What's end event and end execution ID do? How is the cacert option has to do with this change? Proxy timeout? There's a lot of dial that are touched but there's no comments and no description what these options are fixing.

[m4dcoder]

Thank you for including detail explanation in the comments of this PR. This goes a long way to help reviewer understand the context of the change(s).

What happens if something is wrong and the expected end event and end execution ID never arrives? Will the client connection time out? What will the client output looks like in this case?

[guzzijones]

Thank you for including detail explanation in the comments of this PR. This goes a long way to help reviewer understand the context of the change(s).

What happens if something is wrong and the expected end event and end execution ID never arrives? Will the client connection time out? What will the client output looks like in this case?

You would get the same UX as the current implementation of code without this change. The client will hang on writing the results to stdout and you will need to kill the command with ctrl-c.

[m4dcoder]

@guzzijones Thanks for the clarification. Ok with the fix as the events cover most completed statuses and failure case when there's a connection issue or other unforeseen errors.

[m4dcoder]

@guzzijones Thank you for contributing this fix.

[arm4b]

Just a reminder that we'll need a Changelog for this fix before merging.

[guzzijones]

Ok will do.

…

On Sat, Nov 14, 2020, 7:57 AM Eugen Cusmaunsa ***@***.***> wrote: ***@***.**** requested changes on this pull request. Just a reminder that we'll need a Changelog for this fix before merging. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5042 (review)>, or unsubscribe </~https://github.com/notifications/unsubscribe-auth/ACZ5TINZWA44S5VV5VOIFMDSPZ5FJANCNFSM4RPL5UTA> .