Wrap the event stream generator into a function in order to run the initialization code immediately

[wolneykien]

Hi! It seems logical to execute the POST request immediately, when client.events.subscribe() is called instead of postpone it in the generator object. The current version behaves like this:

1. stream = client.events.subscribe(...) (nothing happens!)
2. for e in stream: ... (it fires the request and iterate over the result)

With the proposed version it is possible to:

1. stream = client.events.subscribe(...) (connect, send the request, get 200 OK...)
2. for e in stream: ... (process the events in a loop...)

And it's possible to repeat the step 1 if it fails due to a connection error, etc.

[joni1993]

Hi, thanks for your improvement. I am a bit cautious because it changes the behaviour and may affect other users of this library. What do you think about making a dedicated function that wraps the existing one, to keep current behaviour for backwards compatibility?

[wolneykien]

Mmm... .subscribe_now() ? :-)

[wolneykien]

It's really great that you care about the users. However, I don't think someone really needs to postpone the request. That behaviour was very un-expected to me…

[joni1993]

I totally agree that this is rather unexpected - even for me... 🙃 - i also agree that probably nobody needs to postpone the request - but i am rather worried about existing implementations using the library.

And it's possible to repeat the step 1 if it fails due to a connection error, etc.

If the current code actually behaves like you described (i will check that myself too) then I am looking at the following case specifically:
If an error occurs currently it will probably show up when iterating (step 2) and after your change it will throw on subscribe (step 1). If someone's application using the library only try/catches on the loop (step 2) - it will break their code because it will not catch the Icinga2ApiRequestException anymore.

[wolneykien]

If someone's application using the library only try/catches on the loop...

Yes, in that particular case it would break. So, what? subscribe_now()? Or some option to select the behaviour?

[joni1993]

Option with current behaviour as default sounds great.