Skip to content

Commit

Permalink
Fix log bug.
Browse files Browse the repository at this point in the history
I introduced a bug that caused pypiper to not
output logs. This fixes that bug, and simplifies
the definition of interactive_mode. it also
fixes a small issue with files named 'devnull'.
  • Loading branch information
nsheff committed Jul 14, 2016
1 parent 0e5e538 commit 9a23c42
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 26 deletions.
61 changes: 36 additions & 25 deletions pypiper/pypiper.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,39 +177,49 @@ def _ignore_interrupts(self):
signal.signal(signal.SIGINT, signal.SIG_IGN)
signal.signal(signal.SIGTERM, signal.SIG_IGN)

def start_pipeline(self, args=None, multi=False):
def start_pipeline(self, args = None, multi = False):
"""
Initialize setup. Do some setup, like tee output, print some diagnostics, create temp files.
You provide only the output directory (used for pipeline stats, log, and status flag files).
"""
# Perhaps this could all just be put into __init__, but I just kind of like the idea of a start function
self.make_sure_path_exists(self.pipeline_outfolder)

if not hasattr(__main__, "__file__"):
print("Warning: You're running an interactive python session. This works, but pypiper cannot tee\
the output, so results are only logged to screen.")
# By default, Pypiper will mirror every operation so it is displayed both
# on sys.stdout **and** to a log file. Unfortunately, interactive python sessions
# ruin this by interfering with stdout. So, for interactive mode, we do not enable
# the tee subprocess, sending all output to screen only.
# Starting multiple PipelineManagers in the same script has the same problem, and
# must therefore be run in interactive_mode.

interactive_mode = False # Defaults to off

if multi or not hasattr(__main__, "__file__"):
print("Warning: You're running an interactive python session. This works, but pypiper cannot tee\
the output, so results are only logged to screen.")

# Mirror every operation on sys.stdout to log file
if not multi and not hasattr(__main__, "__file__"):
interactive_mode = True

if not interactive_mode:
sys.stdout = os.fdopen(sys.stdout.fileno(), 'w', 0) # Unbuffer output

# Before creating the subprocess, we use the signal module to ignore TERM and INT signals;
# The subprocess will inherit these settings, so it won't be terminated by INT or TERM
# automatically; but instead, I will clean up this process in the signal handler functions.
# This is required because otherwise, the tee is terminated early, before I have a chance to
# print some final output, and so those things don't end up in the log files because the tee
# subprocess is dead.
# The tee subprocess must be instructed to ignore TERM and INT signals;
# Instead, I will clean up this process in the signal handler functions.
# This is required because otherwise, if pypiper recieves a TERM or INT,
# the tee will be automatically terminated by python before I have a chance to
# print some final output (for example, about when the process stopped),
# and so those things don't end up in the log files because the tee
# subprocess is dead. Instead, I will handle the killing of the tee process
# manually (in the exit handler).

# a for append to file

tee = subprocess.Popen(
["tee", "-a", self.pipeline_log_file], stdin=subprocess.PIPE,
preexec_fn=self._ignore_interrupts)

# Reset the signal handlers after spawning that process
# signal.signal(signal.SIGINT, self._signal_int_handler)
# signal.signal(signal.SIGTERM, self._signal_term_handler)
# In case this pipeline process is terminated with SIGTERM, make sure we kill this spawned process as well.
# If the pipeline is terminated with SIGTERM/SIGINT,
# make sure we kill this spawned tee subprocess as well.
atexit.register(self._kill_child_process, tee.pid)
os.dup2(tee.stdin.fileno(), sys.stdout.fileno())
os.dup2(tee.stdin.fileno(), sys.stderr.fileno())
Expand All @@ -229,21 +239,22 @@ def start_pipeline(self, args=None, multi=False):
gitvars = {}
try:
gitvars['pypiper_dir'] = os.path.dirname(os.path.realpath(__file__))
gitvars['pypiper_hash'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(__file__)) + "; git rev-parse --verify HEAD 2>\\dev\\null", shell=True)
gitvars['pypiper_date'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(__file__)) + "; git show -s --format=%ai HEAD 2>\\dev\\null", shell=True)
gitvars['pypiper_diff'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(__file__)) + "; git diff --shortstat HEAD 2>\\dev\\null", shell=True)
gitvars['pypiper_branch'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(__file__)) + "; git branch | grep '*' 2>\\dev\\null", shell=True)
print("cd " + os.path.dirname(os.path.realpath(__file__)) + "; git rev-parse --verify HEAD 2>/dev/null")
gitvars['pypiper_hash'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(__file__)) + "; git rev-parse --verify HEAD 2>/dev/null", shell=True)
gitvars['pypiper_date'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(__file__)) + "; git show -s --format=%ai HEAD 2>/dev/null", shell=True)
gitvars['pypiper_diff'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(__file__)) + "; git diff --shortstat HEAD 2>/dev/null", shell=True)
gitvars['pypiper_branch'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(__file__)) + "; git branch | grep '*' 2>/dev/null", shell=True)
except Exception:
pass
try:
gitvars['pipe_dir'] = os.path.dirname(os.path.realpath(sys.argv[0]))
gitvars['pipe_hash'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(sys.argv[0])) + "; git rev-parse --verify HEAD 2>\\dev\\null", shell=True)
gitvars['pipe_date'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(sys.argv[0])) + "; git show -s --format=%ai HEAD 2>\\dev\\null", shell=True)
gitvars['pipe_diff'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(sys.argv[0])) + "; git diff --shortstat HEAD 2>\\dev\\null", shell=True)
gitvars['pipe_branch'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(sys.argv[0])) + "; git branch | grep '*' 2>\\dev\\null", shell=True)
gitvars['pipe_hash'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(sys.argv[0])) + "; git rev-parse --verify HEAD 2>/dev/null", shell=True)
gitvars['pipe_date'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(sys.argv[0])) + "; git show -s --format=%ai HEAD 2>/dev/null", shell=True)
gitvars['pipe_diff'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(sys.argv[0])) + "; git diff --shortstat HEAD 2>/dev/null", shell=True)
gitvars['pipe_branch'] = subprocess.check_output("cd " + os.path.dirname(os.path.realpath(sys.argv[0])) + "; git branch | grep '*' 2>/dev/null", shell=True)
except Exception:
pass

# Print out a header section in the pipeline log:
# Wrap things in backticks to prevent markdown from interpreting underscores as emphasis.
print("----------------------------------------")
Expand Down
3 changes: 2 additions & 1 deletion test_pypiper.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class PypiperTest(unittest.TestCase):
def setUp(self):
print("Setting up...")
# Create a fixture
self.pp = pypiper.PipelineManager(name="sample_pipeline", outfolder="pipeline_output/", multi=True)
self.pp = pypiper.PipelineManager(name="sample_pipeline", outfolder="pipeline_output/", multi = False)
self.pp2 = pypiper.PipelineManager(name="sample_pipeline2", outfolder="pipeline_output/", multi=True)

def tearDown(self):
Expand All @@ -38,6 +38,7 @@ def test_me(self):
self.assertEqual(self.pp2.pipeline_name, "sample_pipeline2")
# it creates an outfolder
self.assertTrue(os.path.exists(self.pp.pipeline_outfolder))
self.assertTrue(os.path.isfile(self.pp.pipeline_outfolder + "sample_pipeline_log.md"))

print("Testing status flags...")
self.pp.set_status_flag("testing")
Expand Down

3 comments on commit 9a23c42

@afrendeiro
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great, thanks!
Just tested with microtest.

@nsheff
Copy link
Member Author

@nsheff nsheff commented on 9a23c42 Jul 15, 2016

Choose a reason for hiding this comment

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

I also added a unit test for this so it won't happen again.

@nsheff
Copy link
Member Author

@nsheff nsheff commented on 9a23c42 Jul 15, 2016

Choose a reason for hiding this comment

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

By the way, the upshot of this is that: interactive_mode is now automatically detected, so you don't have to pass multi = True when using the python console anymore; it will detect it automatically.

Please sign in to comment.