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

[MRG+2] Reset terminal colors in external_program ENVIRONMENT print #1742

Merged
merged 1 commit into from
Jul 5, 2016

Conversation

gte620v
Copy link
Contributor

@gte620v gte620v commented Jun 30, 2016

Description

When an ExternalProgramRunError is printed and there are color values in the ENVIRONMENT, the terminal colors are permanently changed. I have noticed this when a PySparkTask fails on AWS EMR. Annoyingly, in EMR, the terminal color is switched to a hard-to-read light blue.

Have you tested this? If so, how?

Yeah, it is a one-line change that works fine.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @ehdr, @erikbern and @adamchainz to be potential reviewers

@codecov-io
Copy link

Current coverage is 76.23%

Merging #1742 into master will decrease coverage by 2.68%

@@             master      #1742   diff @@
==========================================
  Files            95         95          
  Lines         10341      10342     +1   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           8161       7884   -277   
- Misses         2180       2458   +278   
  Partials          0          0          

Powered by Codecov. Last updated by c311975...9301f72

@adamchainz
Copy link
Contributor

I recuse as a reviewer, I have no idea what's happening

@Tarrasch
Copy link
Contributor

Tarrasch commented Jul 1, 2016

LGTM

@ehdr
Copy link
Contributor

ehdr commented Jul 2, 2016

👍

@gte620v gte620v changed the title [MRG] Reset terminal colors in external_program ENVIRONMENT print [MRG+2] Reset terminal colors in external_program ENVIRONMENT print Jul 2, 2016
@Tarrasch Tarrasch merged commit 11a431d into spotify:master Jul 5, 2016
@Tarrasch
Copy link
Contributor

Tarrasch commented Jul 5, 2016

Thanks! :)

@gte620v, oh you're affiliated with Georgia Tech! Cool, I studied abroad there a few years ago. ^^

@gte620v gte620v deleted the reset_terminal_colors branch July 5, 2016 14:31
@gte620v
Copy link
Contributor Author

gte620v commented Jul 5, 2016

Nice! Thanks!

Go Jackets!

p7k pushed a commit to Celmatix/luigi that referenced this pull request Jul 11, 2016
* spotify/master: (25 commits)
  Version 2.2.0
  Add tests for hashing parameters (spotify#1719)
  Update call to iteritems in luigi/tools/deps: deprecated in Python 3 (spotify#1749)
  Reset terminal colors in external_program (spotify#1742)
  Caches get_autoconfig_client on a per-thread basis
  Fix bug with GCSFlagTarget
  Add additional event handlers to tasks (spotify#1698)
  Reduce number of get_params calls in common_params.
  Removes redundant function definitions from rpc and server (spotify#1734)
  Fix salesforce default content type (spotify#1724)
  Rename MockTarget class variable _fn to path
  Remove MockTarget path property
  Deprecated LocalTarget fn propery
  Add note about underscore in parameter names (spotify#1729)
  Remove tracking url callback hack (spotify#1722)
  Consistent Luigi spelling in docs (spotify#1723)
  Update example_top_artists.rst (spotify#1662)
  Add combiner to docstrings in mrrunner
  Add luigi-deps-tree visualising tool (spotify#1680)
  Adding release step for Debian packages. (spotify#1718)
  ...
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.

6 participants