fix: Add a optional grace time period to shutdown method #40
Conversation
| """Shutdown the client instance, not accepting new events, flush all events in buffer""" | ||
| self.configuration.opt_out = True | ||
| self.__timeline.shutdown() | ||
| if grace_time_milliseconds > 0: |
There was a problem hiding this comment.
Thanks for the quick update! Not sure whether we should add this here, as if shutdown is exposed as public API, user can do the time.sleep if they need. Do we consider to use async tasks for the worker? https://docs.python.org/3/library/asyncio-task.html.
There was a problem hiding this comment.
The current worker use threading. Switch to coroutine may need some work to re-design and implement the retry logic. shutdown() is a public interface suppose to be called before application exit. I have run some tests. Add ether flush or shutdown after tracking will stop the error of can not schedule future. Let me ask customer if they added these call.
from amplitude import *
client = Amplitude('')
client.track(BaseEvent(
event_type='Python Event',
user_id='[email protected]',
ip='127.0.0.1',
event_properties={
'keyString': 'valueString',
'keyInt': 11,
'keyBool': True
}
))
# client.flush()
client.shutdown()```
There was a problem hiding this comment.
I see, I thought this fix is for the other issue (timing issue). I cannot reproduce the error can not schedule future in both Python 3.9 and 3.10. If adding this can fix the issue, I am good with change. Just to double check, will this be stable with the usage of sleep?
There was a problem hiding this comment.
@bohan-amplitude Can you please first enable py37-p311 pipelines, so we can see what really happens with different python versions, running only on py36 is insane, that version pinned for the fjords long time ago.
|
I added #41 but apparently somene is really afraid about incomming pull requests... to the extent of requiring approval ;) |
|
Closing this one for now since we pushed a fix in v1.1.1. Feel free to try the new version and let us know if the experience is improved or not. Thanks. |
Summary
Add optional parameter: grace_time_milliseconds to client.shutdown()
Allow user to delay main interpreter thread close waiting the last batch events task scheduled to threads pool.
Checklist