Timed history save #234

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
2 participants
@mchandra

Implemented autosave by spawning a thread which saves periodically. Thread spawned when core/InteractiveShell is initialized and is closed when ctrl+D is pressed. If the user presses 'n' when asked to exit the thread is spawned again.

@minrk

This comment has been minimized.

Show comment Hide comment
@minrk

minrk Dec 19, 2010

Member

This sounds great, but I have a few notes:

  • It seems like the HistorySaveThread object should be defined in core.history instead of core.interactiveshell

  • Saving every second seems excessive, especially if you develop a large history and/or leave an IPython session sitting in the background (many do this). Perhaps make it every 5 or 10, or even every minute. If you are going to save at high frequency, it's important to detect changes in order to avoid writing the exact same content to a file over and over again, and we don't do this yet.

  • If you do increase the save interval (and possibly even if not), we don't want the save dialog to wait for the interval to complete to come up, and that's what currently happens. You could change:

    self.shell.history_thread.exit_now=True
    self.shell.history_thread.join()
    if self.confirm_exit:
        if self.ask_yes_no('Do you really want to exit ([y]/n)?','y'):
            self.ask_exit()
        else:
            self.shell.history_thread = HistorySaveThread(self.shell, 1,
                    False)
            self.shell.history_thread.start()
    else:
        self.ask_exit()
    

to

    if self.confirm_exit:
        if self.ask_yes_no('Do you really want to exit ([y]/n)?','y'):
            self.shell.history_thread.exit_now=True
            self.shell.history_thread.join()
            self.ask_exit()
    else:
        self.shell.history_thread.exit_now=True
        self.shell.history_thread.join()
        self.ask_exit()

You can also use a threading.Condition, and its wait()/notify() methods to avoid having to wait on join() at all.

Member

minrk commented Dec 19, 2010

This sounds great, but I have a few notes:

  • It seems like the HistorySaveThread object should be defined in core.history instead of core.interactiveshell

  • Saving every second seems excessive, especially if you develop a large history and/or leave an IPython session sitting in the background (many do this). Perhaps make it every 5 or 10, or even every minute. If you are going to save at high frequency, it's important to detect changes in order to avoid writing the exact same content to a file over and over again, and we don't do this yet.

  • If you do increase the save interval (and possibly even if not), we don't want the save dialog to wait for the interval to complete to come up, and that's what currently happens. You could change:

    self.shell.history_thread.exit_now=True
    self.shell.history_thread.join()
    if self.confirm_exit:
        if self.ask_yes_no('Do you really want to exit ([y]/n)?','y'):
            self.ask_exit()
        else:
            self.shell.history_thread = HistorySaveThread(self.shell, 1,
                    False)
            self.shell.history_thread.start()
    else:
        self.ask_exit()
    

to

    if self.confirm_exit:
        if self.ask_yes_no('Do you really want to exit ([y]/n)?','y'):
            self.shell.history_thread.exit_now=True
            self.shell.history_thread.join()
            self.ask_exit()
    else:
        self.shell.history_thread.exit_now=True
        self.shell.history_thread.join()
        self.ask_exit()

You can also use a threading.Condition, and its wait()/notify() methods to avoid having to wait on join() at all.

@mchandra

This comment has been minimized.

Show comment Hide comment
@mchandra

mchandra Dec 20, 2010

Hi, thanks for the suggestions. I have implemented it as you said but I have a few doubts. I'm looking at how to use the threading.Condition and wait()/noitfy() methods but meanwhile I realized that I do not need to use join() and the results are still the same. The problem with the current implementation is that the user has to wait for the sleep time to complete when ctrl+D + yes is pressed before the program can exit. Any suggestions on how to make the program exit instantly? Perhaps this is possible with the methods you suggested and I'm working on how to use them.

Hi, thanks for the suggestions. I have implemented it as you said but I have a few doubts. I'm looking at how to use the threading.Condition and wait()/noitfy() methods but meanwhile I realized that I do not need to use join() and the results are still the same. The problem with the current implementation is that the user has to wait for the sleep time to complete when ctrl+D + yes is pressed before the program can exit. Any suggestions on how to make the program exit instantly? Perhaps this is possible with the methods you suggested and I'm working on how to use them.

@minrk

This comment has been minimized.

Show comment Hide comment
@minrk

minrk Dec 20, 2010

Member

I think you can use wait(timeout) instead of sleep(timeout), then the notify() trigger will effectively interrupt the sleep. Along the lines of:
...
while True:
self.cond.acquire()
self.cond.wait(self.time_interval)
self.cond.release()
if self.exit_now == True:
break
...
and trigger with something like:

history_thread.exit_now = True
history_thread.cond.acquire()
history_thread.cond.notify()
history_thread.cond.release()
Member

minrk commented Dec 20, 2010

I think you can use wait(timeout) instead of sleep(timeout), then the notify() trigger will effectively interrupt the sleep. Along the lines of:
...
while True:
self.cond.acquire()
self.cond.wait(self.time_interval)
self.cond.release()
if self.exit_now == True:
break
...
and trigger with something like:

history_thread.exit_now = True
history_thread.cond.acquire()
history_thread.cond.notify()
history_thread.cond.release()
@mchandra

This comment has been minimized.

Show comment Hide comment
@mchandra

mchandra Dec 21, 2010

Hi, I made the changes and now it works nicely. I also increased the timeout to one minute.

Hi, I made the changes and now it works nicely. I also increased the timeout to one minute.

@minrk

This comment has been minimized.

Show comment Hide comment
@minrk

minrk Dec 21, 2010

Member

Great, this looks excellent. One last question: Why do you start a new HistorySaveThread if confirm_exit is False? Shouldn't it just go straight to ask_exit()?

Member

minrk commented Dec 21, 2010

Great, this looks excellent. One last question: Why do you start a new HistorySaveThread if confirm_exit is False? Shouldn't it just go straight to ask_exit()?

@mchandra

This comment has been minimized.

Show comment Hide comment
@mchandra

mchandra Dec 21, 2010

Oops. You're right, those lines shouldn't be there. There were part of earlier code where I was using slightly different logic. Fixed it now.

Oops. You're right, those lines shouldn't be there. There were part of earlier code where I was using slightly different logic. Fixed it now.

@mchandra

This comment has been minimized.

Show comment Hide comment
@mchandra

mchandra Dec 25, 2010

Hi, so what happens now?

Hi, so what happens now?

@mchandra

This comment has been minimized.

Show comment Hide comment
@mchandra

mchandra Dec 26, 2010

Periodic Autosave added in background thread

closed by 8bac8cb

Signed-off-by: MinRK <benjaminrk@gmail.com>

Periodic Autosave added in background thread

closed by 8bac8cb

Signed-off-by: MinRK <benjaminrk@gmail.com>

@minrk

This comment has been minimized.

Show comment Hide comment
@minrk

minrk Dec 26, 2010

Member

Hi, sorry for the delay.

It's merged into master now.

Member

minrk commented Dec 26, 2010

Hi, sorry for the delay.

It's merged into master now.

markvoorhies pushed a commit to markvoorhies/ipython that referenced this pull request Apr 21, 2011

Periodic Autosave added in background thread
closes gh-234

Signed-off-by: MinRK <benjaminrk@gmail.com>

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Periodic Autosave added in background thread
closes gh-234

Signed-off-by: MinRK <benjaminrk@gmail.com>

This issue was closed.

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