8000 Add a "capture_output" parameter to ExternalProgramTask by jytug · Pull Request #2430 · spotify/luigi · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add a "capture_output" parameter to ExternalProgramTask #2430

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 6, 2018
Merged

Add a "capture_output" parameter to ExternalProgramTask #2430

merged 1 commit into from
Sep 6, 2018

Conversation

jytug
Copy link
Contributor
@jytug jytug commented May 25, 2018 •
edited
Loading

Description

A capture_output is added to ExternalProgramTask, so that
the output is captured only optionally (which is still the default
behaviour).

Motivation and Context

This change can make debugging long-running tasks easier
by allowing the user to see the output on the fly. The current
behaviour remains unchanged by having the parameter set
to True by default

Have you tested this? If so, how?

I have been running jobs this way by hot-fixing luigi
in dist-packages

Simple usage:

In [1]: from luigi.contrib.external_program import ExternalProgramTask

In [2]: class LsTask(ExternalProgramTask):
   ...:     def program_args(self):
   ...:         return ['ls']
   ...:     

In [3]: task = LsTask(capture_output=False)

In [4]: task.run()
bin  codecov.yml  CONTRIBUTING.rst  doc  examples  ISSUE_TEMPLATE.md  LICENSE  luigi  MANIFEST.in  PULL_REQUEST_TEMPLATE.md  README.rst  RELEASE-PROCESS.rst  scripts  setup.py  test  tox.in

Edit:
in reference to issue #2342

@Tarrasch
Copy link
Contributor

Good idea. I would like to see some docs and tests though. :)

@jytug
Copy link
Contributor Author
jytug commented Jun 5, 2018 •
edited
Loading

I would like to add a unit test in tests/contrib/external_program_test.py. However, I can't seem to find a way to run those tests. An invocation like:

python -m unittest test/contrib/external_program_test.py

results in the following error:

ImportError: Failed to import test module: contrib
Traceback (most recent call last):
  File "/Users/filip/miniconda2/envs/p3/lib/python3.6/unittest/loader.py", line 153, in loadTestsFromName
    module = __import__(module_name)
ModuleNotFoundError: No module named 'test.contrib'

@dlstadther
Copy link
Collaborator

@jytug take a look at CONTRIBUTING.rst

@jytug jytug force-pushed the optional-output-capture branch 2 times, most recently from 7d35713 to 6a0df94 Compare June 7, 2018 13:52
@jytug
Add a "capture_output" parameter to ExternalProgramTask
d0ab93f 
This allows, among others, for seeing the output of
long-running tasks in the console. The parameter defaults
to True.

A unit test has been added to test/contrib/external_program_test.py,
as well as a few lines in the docstring regarding the attribute
@jytug jytug force-pushed the optional-output-capture branch from 6a0df94 to d0ab93f Compare June 9, 2018 16:45
dlstadther
dlstadther approved these changes Sep 6, 2018
View reviewed changes
Copy link
Collaborator
@dlstadther dlstadther left a comment

Choose a reason for hiding this comment

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

Somehow this rebase (with test added) escaped my notifications.

LGTM

@dlstadther dlstadther merged commit 718a48a into spotify:master Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dlstadther dlstadther dlstadther approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jytug @Tarrasch @dlstadther
0