Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include sys.argv in error emails #1637

Merged
merged 2 commits into from
Apr 8, 2016
Merged

Conversation

neilisaac
Copy link
Contributor

Sometimes my team shares base tasks. When we get an error email, it's sometimes unclear which upstream task requires() the one that failed.

Following @Tarrasch's suggestion of adding command line arguments to address this.

@neilisaac
Copy link
Contributor Author

Example email body:

A task failed when running. Most likely run() raised an exception.

Name: SubTask

Parameters:
  p: some option

Command line:
  'testtask.py' '--local-scheduler' 'TestTask' '--p' 'asdf'

Runtime error:
Traceback (most recent call last):
  File "/home/nisaac/code/luigi/luigi/worker.py", line 172, in run
    new_deps = self._run_get_new_deps()
  File "/home/nisaac/code/luigi/luigi/worker.py", line 114, in _run_get_new_deps
    task_gen = self.task.run()
  File "testtask.py", line 9, in run
    raise Exception('Intentional failure')
Exception: Intentional failure

@@ -509,7 +510,8 @@ def _email_task_failure(self, task, formatted_traceback):

def _email_error(self, task, formatted_traceback, subject, headline):
formatted_subject = subject.format(task=task, host=self.host)
message = notifications.format_task_error(headline, task, formatted_traceback)
command = ' '.join("'{}'".format(arg) for arg in sys.argv)
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes the command look weird and hard to read I think. What about simply

command = subprocess.list2cmdline(sys.argv)

@Tarrasch
Copy link
Contributor

Tarrasch commented Apr 8, 2016

LGTM. But the formatting looks strange to me. See my inline comment suggestion. I would rather just see:

  testtask.py --local-scheduler TestTask --p asdf

@Tarrasch
Copy link
Contributor

Tarrasch commented Apr 8, 2016

LGTM! Will merge on green :)

@Tarrasch Tarrasch merged commit ebab5b2 into spotify:master Apr 8, 2016
This was referenced Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants