You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/04/16 13:28:46 UTC
[GitHub] [airflow] mik-laj opened a new pull request #8404: Add colors to
airflow config command
mik-laj opened a new pull request #8404: Add colors to airflow config command
URL: https://github.com/apache/airflow/pull/8404
I didn't write automatic tests because it is a system-specific thing and we can't write reliable automated tests. Argument parsing is covered by current tests.
I did manual tests.
When I run the `airflow config` command, I see the screen below.
<img width="549" alt="Screenshot 2020-04-16 at 15 19 46" src="https://user-images.githubusercontent.com/12058428/79461334-51d7c900-7ff6-11ea-8170-40cfe549ce99.png">
When I run the `airflow config | cat` command, I see the scrceen below.
<img width="543" alt="Screenshot 2020-04-16 at 15 27 45" src="https://user-images.githubusercontent.com/12058428/79461745-dd515a00-7ff6-11ea-8baa-9a297c86d041.png">
---
Make sure to mark the boxes below before creating PR: [x]
- [X] Description above provides context of the change
- [X] Unit tests coverage for changes (not needed for documentation changes)
- [X] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [X] Relevant documentation is updated including usage instructions.
- [X] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #8404: Add colors to airflow
config command
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #8404: Add colors to airflow config command
URL: https://github.com/apache/airflow/pull/8404#issuecomment-614663879
Ah that's better, and more "gnu"-like for `ls --color=auto` etc. Nice.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj merged pull request #8404: Add colors to airflow
config command
Posted by GitBox <gi...@apache.org>.
mik-laj merged pull request #8404: Add colors to airflow config command
URL: https://github.com/apache/airflow/pull/8404
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #8404: Add
colors to airflow config command
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #8404: Add colors to airflow config command
URL: https://github.com/apache/airflow/pull/8404#discussion_r409564450
##########
File path: airflow/cli/cli_parser.py
##########
@@ -153,6 +153,12 @@ def __init__(self, flags=None, help=None, action=None, default=None, nargs=None,
),
choices=tabulate_formats,
default="fancy_grid")
+ARG_COLOR = Arg(
+ ('--color',),
+ help="do emit colored output (default: auto-detect)")
+ARG_NO_COLOR = Arg(
+ ('--no-color',),
+ help="do not emit colored output (default: auto-detect)")
Review comment:
```suggestion
ARG_COLOR = Arg(
('--color',),
help="do emit colored output (default: auto-detect)",
action="store_true")
ARG_NO_COLOR = Arg(
('--no-color',),
help="do not emit colored output (default: auto-detect)",
action="store_false",
dest="color")
```
And then should_use_colors is a bit simpler.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #8404: Add
colors to airflow config command
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8404: Add colors to airflow config command
URL: https://github.com/apache/airflow/pull/8404#discussion_r409561711
##########
File path: airflow/cli/cli_parser.py
##########
@@ -153,6 +153,12 @@ def __init__(self, flags=None, help=None, action=None, default=None, nargs=None,
),
choices=tabulate_formats,
default="fancy_grid")
+ARG_COLOR = Arg(
+ ('--color',),
+ help="do emit colored output (default: auto-detect)")
+ARG_NO_COLOR = Arg(
+ ('--no-color',),
Review comment:
Oh yes. `choices`. I will update PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #8404: Add
colors to airflow config command
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #8404: Add colors to airflow config command
URL: https://github.com/apache/airflow/pull/8404#discussion_r409563399
##########
File path: airflow/cli/cli_parser.py
##########
@@ -153,6 +153,12 @@ def __init__(self, flags=None, help=None, action=None, default=None, nargs=None,
),
choices=tabulate_formats,
default="fancy_grid")
+ARG_COLOR = Arg(
+ ('--color',),
+ help="do emit colored output (default: auto-detect)")
+ARG_NO_COLOR = Arg(
+ ('--no-color',),
Review comment:
Nope, I'm making that up.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #8404: Add
colors to airflow config command
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #8404: Add colors to airflow config command
URL: https://github.com/apache/airflow/pull/8404#discussion_r409564647
##########
File path: airflow/utils/cli.py
##########
@@ -234,3 +234,39 @@ def sigquit_handler(sig, frame): # pylint: disable=unused-argument
if line:
code.append(" {}".format(line.strip()))
print("\n".join(code))
+
+
+def is_terminal_support_colors() -> bool:
+ """"
+ Try to determine if the current terminal supports colors.
+ """
+ if sys.platform == 'win32':
+ return False
+ if not hasattr(sys.stdout, 'isatty'):
+ return False
+ if not sys.stdout.isatty():
+ return False
+ if 'COLORTERM' in os.environ:
+ return True
+ term = os.environ.get('TERM', 'dumb').lower()
+ if term in ('xterm', 'linux') or 'color' in term:
+ return True
+ return False
+
+
+def should_use_colors(args) -> bool:
+ """
+ Processes arguments and decides whether to enable color in output
+ """
+ if args.color and args.no_color:
Review comment:
https://docs.python.org/3/library/argparse.html#mutual-exclusion
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #8404: Add
colors to airflow config command
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8404: Add colors to airflow config command
URL: https://github.com/apache/airflow/pull/8404#discussion_r409570732
##########
File path: airflow/utils/cli.py
##########
@@ -234,3 +234,39 @@ def sigquit_handler(sig, frame): # pylint: disable=unused-argument
if line:
code.append(" {}".format(line.strip()))
print("\n".join(code))
+
+
+def is_terminal_support_colors() -> bool:
+ """"
+ Try to determine if the current terminal supports colors.
+ """
+ if sys.platform == 'win32':
+ return False
+ if not hasattr(sys.stdout, 'isatty'):
+ return False
+ if not sys.stdout.isatty():
+ return False
+ if 'COLORTERM' in os.environ:
+ return True
+ term = os.environ.get('TERM', 'dumb').lower()
+ if term in ('xterm', 'linux') or 'color' in term:
Review comment:
Breeze uses ``xterm``. I tested on Mac OS and Breeze.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #8404: Add
colors to airflow config command
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #8404: Add colors to airflow config command
URL: https://github.com/apache/airflow/pull/8404#discussion_r409563276
##########
File path: airflow/utils/cli.py
##########
@@ -234,3 +234,39 @@ def sigquit_handler(sig, frame): # pylint: disable=unused-argument
if line:
code.append(" {}".format(line.strip()))
print("\n".join(code))
+
+
+def is_terminal_support_colors() -> bool:
+ """"
+ Try to determine if the current terminal supports colors.
+ """
+ if sys.platform == 'win32':
+ return False
+ if not hasattr(sys.stdout, 'isatty'):
+ return False
+ if not sys.stdout.isatty():
+ return False
+ if 'COLORTERM' in os.environ:
+ return True
+ term = os.environ.get('TERM', 'dumb').lower()
+ if term in ('xterm', 'linux') or 'color' in term:
Review comment:
_Strictly_ speaking `TERM=xterm` doesn't support color. This is probably fine :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #8404: Add
colors to airflow config command
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #8404: Add colors to airflow config command
URL: https://github.com/apache/airflow/pull/8404#discussion_r409560503
##########
File path: airflow/cli/cli_parser.py
##########
@@ -153,6 +153,12 @@ def __init__(self, flags=None, help=None, action=None, default=None, nargs=None,
),
choices=tabulate_formats,
default="fancy_grid")
+ARG_COLOR = Arg(
+ ('--color',),
+ help="do emit colored output (default: auto-detect)")
+ARG_NO_COLOR = Arg(
+ ('--no-color',),
Review comment:
I think it's possible to do this in ArgParse without a separate option. One moment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services