Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

Automatic duckpan app restart on IA update #196

Merged
merged 7 commits into from
Jan 9, 2015

Conversation

zachthompson
Copy link
Contributor

Which issues (if any) does this change solve? (please link them here)
#52

Briefly describe your fix. Are there any important details we should know about?
Alternate strategy to #175 code reloading. Adds the role App::DuckPAN::Restart which restarts the running duckpan application automatically when instant answer code is changed.

Incorporates #190 as well since the current query console does not exit cleanly when killed.

…es in the development hierarchy.

Apps can declare _run_app() in their package and use App::DuckPAN::Restart.  Changes should be as easy as:
1. use Moo;
   with qw(App::DuckPAN::Restart);
2. Rename run() _run_app()
3. sub run {
      ....
      $self->run_restarter(\@Args);
   }
@nilnilnil
Copy link
Contributor

👍

@nilnilnil
Copy link
Contributor

Failing tests though.

$self->_run_app($args);
exit 0;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment why this is here -- based on our discussion it's because you want to reap the $app if $fmon fails to fork.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. We need to clean up the spawned app before exiting with the second fork. If we want the structure to be the same, we can change the first fork to the latter's.

@nilnilnil
Copy link
Contributor

@zachthompson still needs merge conflict resolution

}

# wait for one them to exit. -1 waits for all children
my $pid = waitpid -1, 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there might be a slight race condition here when outside of an IA directory if the pid for the fmon is reaped first. Let me have a look at it. I test this very condition this morning and it was fine but running it this afternoon for something unrelated triggered it.

@russellholt
Copy link
Contributor

@zachthompson let me know when you're ready.

@nilnilnil
Copy link
Contributor

@zachthompson are you all set here?

@zachthompson
Copy link
Contributor Author

@nilnilnil @russellholt ready

@nilnilnil
Copy link
Contributor

👍

nilnilnil added a commit that referenced this pull request Jan 9, 2015
Automatic duckpan app restart on IA update
@nilnilnil nilnilnil merged commit 299c9ae into duckduckgo:master Jan 9, 2015
@zachthompson zachthompson deleted the app_restart branch January 9, 2015 20:04
# ABSTRACT: Automatic restarting of application on file change

use File::Find::Rule;
use Filesys::Notify::Simple;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zachthompson These need to be added to the dist.ini please :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants