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/11/29 15:29:05 UTC

[GitHub] [airflow] turbaszek opened a new pull request #12704: Refactor list rendering in commands

turbaszek opened a new pull request #12704:
URL: https://github.com/apache/airflow/pull/12704


   This commit unifies the mechanism of rendering output of tabular
   data. This gives users a possibility to eiter display a tabular
   representation of data or render it as valid json or yaml payload.
   
   Closes: #12699
   
   ## Example:
   Table:
   ```
   root@e794bcc2d698:/opt/airflow# airflow tasks states-for-dag-run tasks_are_awesome 2020-11-13T00:00:00+00:00
   dag_id            | execution_date            | task_id | state   | start_date                       | end_date
   ==================+===========================+=========+=========+==================================+=================================
   tasks_are_awesome | 2020-11-13T00:00:00+00:00 | numbers | success | 2020-11-29T14:53:46.811030+00:00 | 2020-11-29T14:53:46.974545+00:00
   tasks_are_awesome | 2020-11-13T00:00:00+00:00 | show__2 | success | 2020-11-29T14:53:56.926441+00:00 | 2020-11-29T14:53:57.118781+00:00
   tasks_are_awesome | 2020-11-13T00:00:00+00:00 | show    | success | 2020-11-29T14:53:56.915802+00:00 | 2020-11-29T14:53:57.125230+00:00
   tasks_are_awesome | 2020-11-13T00:00:00+00:00 | show__1 | success | 2020-11-29T14:53:56.922131+00:00 | 2020-11-29T14:53:57.129091+00:00
   tasks_are_awesome | 2020-11-13T00:00:00+00:00 | show__3 | success | 2020-11-29T14:53:56.931243+00:00 | 2020-11-29T14:53:57.126306+00:00
   ```
   
   JSON:
   ```
   root@e794bcc2d698:/opt/airflow# airflow tasks states-for-dag-run tasks_are_awesome 2020-11-13T00:00:00+00:00 --outpu json
   [{"dag_id": "tasks_are_awesome", "execution_date": "2020-11-13T00:00:00+00:00", "task_id": "numbers", "state": "success", "start_date": "2020-11-29T14:53:46.811030+00:00", "end_date": "2020-11-29T14:53:46.974545+00:00"}, {"dag_id": "tasks_are_awesome", "execution_date": "2020-11-13T00:00:00+00:00", "task_id": "show__2", "state": "success", "start_date": "2020-11-29T14:53:56.926441+00:00", "end_date": "2020-11-29T14:53:57.118781+00:00"}, {"dag_id": "tasks_are_awesome", "execution_date": "2020-11-13T00:00:00+00:00", "task_id": "show", "state": "success", "start_date": "2020-11-29T14:53:56.915802+00:00", "end_date": "2020-11-29T14:53:57.125230+00:00"}, {"dag_id": "tasks_are_awesome", "execution_date": "2020-11-13T00:00:00+00:00", "task_id": "show__1", "state": "success", "start_date": "2020-11-29T14:53:56.922131+00:00", "end_date": "2020-11-29T14:53:57.129091+00:00"}, {"dag_id": "tasks_are_awesome", "execution_date": "2020-11-13T00:00:00+00:00", "task_id": "show__3", "state": "succes
 s", "start_date": "2020-11-29T14:53:56.931243+00:00", "end_date": "2020-11-29T14:53:57.126306+00:00"}]
   ```
   
   YAML:
   ```
   root@e794bcc2d698:/opt/airflow# airflow tasks states-for-dag-run tasks_are_awesome 2020-11-13T00:00:00+00:00 --output yaml
   - dag_id: tasks_are_awesome
     end_date: '2020-11-29T14:53:46.974545+00:00'
     execution_date: '2020-11-13T00:00:00+00:00'
     start_date: '2020-11-29T14:53:46.811030+00:00'
     state: success
     task_id: numbers
   - dag_id: tasks_are_awesome
     end_date: '2020-11-29T14:53:57.118781+00:00'
     execution_date: '2020-11-13T00:00:00+00:00'
     start_date: '2020-11-29T14:53:56.926441+00:00'
     state: success
     task_id: show__2
   - dag_id: tasks_are_awesome
     end_date: '2020-11-29T14:53:57.125230+00:00'
     execution_date: '2020-11-13T00:00:00+00:00'
     start_date: '2020-11-29T14:53:56.915802+00:00'
     state: success
     task_id: show
   - dag_id: tasks_are_awesome
     end_date: '2020-11-29T14:53:57.129091+00:00'
     execution_date: '2020-11-13T00:00:00+00:00'
     start_date: '2020-11-29T14:53:56.922131+00:00'
     state: success
     task_id: show__1
   - dag_id: tasks_are_awesome
     end_date: '2020-11-29T14:53:57.126306+00:00'
     execution_date: '2020-11-13T00:00:00+00:00'
     start_date: '2020-11-29T14:53:56.931243+00:00'
     state: success
     task_id: show__3
   ```
   
   <img width="1378" alt="Screenshot 2020-11-29 at 02 07 29" src="https://user-images.githubusercontent.com/9528307/100546142-f0e8c280-325f-11eb-9ec8-1d001faf7d63.png">
   
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   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).
   


----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek commented on pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#issuecomment-735956020


   @potiuk @mik-laj @kaxil @ashb thoughts?


----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r532868322



##########
File path: airflow/cli/commands/connection_command.py
##########
@@ -19,78 +19,54 @@
 import json
 import os
 import sys
-from typing import List
+from typing import Any, Dict, List
 from urllib.parse import urlparse, urlunparse
 
-import pygments
 import yaml
-from pygments.lexers.data import YamlLexer
 from sqlalchemy.orm import exc
-from tabulate import tabulate
 
+from airflow.cli.simple_table import AirflowConsole
 from airflow.exceptions import AirflowNotFoundException
 from airflow.hooks.base_hook import BaseHook
 from airflow.models import Connection
 from airflow.utils import cli as cli_utils
-from airflow.utils.cli import should_use_colors
-from airflow.utils.code_utils import get_terminal_formatter
+from airflow.utils.cli import suppress_logs_and_warning
 from airflow.utils.session import create_session
 
 
-def _tabulate_connection(conns: List[Connection], tablefmt: str):
-    tabulate_data = [
-        {
-            'Conn Id': conn.conn_id,
-            'Conn Type': conn.conn_type,
-            'Description': conn.description,
-            'Host': conn.host,
-            'Port': conn.port,
-            'Is Encrypted': conn.is_encrypted,
-            'Is Extra Encrypted': conn.is_encrypted,
-            'Extra': conn.extra,
-        }
-        for conn in conns
-    ]
-
-    msg = tabulate(tabulate_data, tablefmt=tablefmt, headers='keys')
-    return msg
-
-
-def _yamulate_connection(conn: Connection):
-    yaml_data = {
-        'Id': conn.id,
-        'Conn Id': conn.conn_id,
-        'Conn Type': conn.conn_type,
-        'Description': conn.description,
-        'Host': conn.host,
-        'Schema': conn.schema,
-        'Login': conn.login,
-        'Password': conn.password,
-        'Port': conn.port,
-        'Is Encrypted': conn.is_encrypted,
-        'Is Extra Encrypted': conn.is_encrypted,
-        'Extra': conn.extra_dejson,
-        'URI': conn.get_uri(),
+def _connection_mapper(conn: Connection) -> Dict[str, Any]:
+    return {
+        'id': conn.id,
+        'conn_id': conn.conn_id,
+        'conn_type': conn.conn_type,
+        'description': conn.description,
+        'host': conn.host,
+        'schema': conn.schema,
+        'login': conn.login,
+        'password': conn.password,
+        'port': conn.port,
+        'is_encrypted': conn.is_encrypted,
+        'is_extra_encrypted': conn.is_encrypted,
+        'extra_dejson': conn.extra_dejson,
+        'get_uri()': conn.get_uri(),

Review comment:
       Done in caf81ff

##########
File path: UPDATING.md
##########
@@ -52,6 +52,39 @@ assists users migrating to a new version.
 
 ## Master
 
+### Changes to output argument in commands
+
+Instead of using [tabulate](https://pypi.org/project/tabulate/) to render commands output
+we use [rich](https://github.com/willmcgugan/rich). Due to this change the `--output` argument
+will no longer accept formats of tabulate tables. Instead it accepts:
+
+- `table` - will render the output in predefined table
+- `json` - will render the output as a json
+- `yaml` - will render the output as yaml
+
+By doing this we increased consistency and gave users possibility to manipulate the
+output programmatically (when using json or yaml).
+
+Affected commands:
+
+- `airflow dags list`
+- `airflow dags report`
+- `airflow dags list-runs`
+- `airflow dags list-jobs`
+- `airflow connections list`
+- `airflow connections get`
+- `airflow pools list`
+- `airflow pools get`
+- `airflow pools set`
+- `airflow pools delete`
+- `airflow pools export`
+- `airflow role list`
+- `airflow providers list`
+- `airflow providers get`
+- `airflow tasks states-for-dag-run`
+- `airflow users list`
+- `airflow variables list`

Review comment:
       Done in caf81ff

##########
File path: docs/usage-cli.rst
##########
@@ -174,3 +174,40 @@ You will see a similar result as in the screenshot below.
 .. figure:: img/usage_cli_imgcat.png
 
     Preview of DAG in iTerm2
+
+Formatting commands output
+--------------------------
+
+Some Airflow commands like ``dags list`` or ``tasks states-for-dag-run`` support ``--output`` flag which allow users

Review comment:
       Done in caf81ff

##########
File path: airflow/cli/simple_table.py
##########
@@ -14,11 +14,78 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+import json
+from typing import Any, Callable, Dict, List, Optional, Union
 
+import yaml
 from rich.box import ASCII_DOUBLE_HEAD
+from rich.console import Console
+from rich.syntax import Syntax
 from rich.table import Table
 
 
+class AirflowConsole(Console):
+    """Airflow rich console"""
+
+    def print_as_json(self, data: Dict):
+        """Renders dict as json text representation"""
+        json_content = json.dumps(data)
+        self.print(Syntax(json_content, "json", theme="ansi_dark"), soft_wrap=True)
+
+    def print_as_yaml(self, data: Dict):
+        """Renders dict as yaml text representation"""
+        yaml_content = yaml.dump(data)
+        self.print(Syntax(yaml_content, "yaml", theme="ansi_dark"), soft_wrap=True)
+
+    def print_as_table(self, data: List[Dict]):
+        """Renders list of dictionaries as table"""
+        if not data:
+            self.print("No data found")
+            return
+
+        table = SimpleTable(
+            show_header=True,
+        )
+        for col in data[0].keys():
+            table.add_column(col)
+
+        for row in data:
+            table.add_row(*[str(d) for d in row.values()])
+        self.print(table)
+
+    def _normalize_data(self, value: Any, output: str) -> Union[list, str, dict]:
+        if isinstance(value, (tuple, list)):
+            if output == "table":
+                return ",".join(self._normalize_data(x, output) for x in value)
+            return [self._normalize_data(x, output) for x in value]
+        if isinstance(value, dict) and output != "table":
+            return {k: self._normalize_data(v, output) for k, v in value.items()}
+        return str(value)
+
+    def print_as(self, data: List[Union[Dict, Any]], output: str, mapper: Optional[Callable] = None):
+        """Prints provided using format specified by output argument"""
+        output_to_renderer = {
+            "json": self.print_as_json,
+            "yaml": self.print_as_yaml,
+            "table": self.print_as_table,
+        }
+        renderer = output_to_renderer.get(output)
+        if not renderer:
+            raise ValueError(
+                f"Unknown formatter: {output}. Allowed options: {list(output_to_renderer.keys())}"
+            )
+
+        if not all(isinstance(d, dict) for d in data) and not mapper:
+            raise ValueError("To tabulate non-dictionary data you need to provider `mapper` function")

Review comment:
       Done in caf81ff




----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek edited a comment on pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#issuecomment-735413321


   The only thing that I'm wondering about is suppressing / removing log records when using json/yaml. Because otherwise the output is not valid. For example:
   ```
   root@e794bcc2d698:/opt/airflow# airflow dags list --output yaml | yq "."
   /opt/airflow/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py:26 DeprecationWarning: This module is deprecated. Please use `kubernetes.client.models.V1Volume`.
   /opt/airflow/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py:27 DeprecationWarning: This module is deprecated. Please use `kubernetes.client.models.V1VolumeMount`.
   yq: Error running jq: ParserError: expected '<document start>', but found '{'
     in "<stdin>", line 1, column 27.
   [
     "2020-11-29T15:41:59",
     435
   ]
   ```
   or
   ```
   ^[[Aroot@e794bcc2d698:/opt/airflow# airflow providers hooks --output yaml | yq ".[]"
   /usr/local/lib/python3.6/site-packages/snowflake/connector/options.py:39 UserWarning: You have an incompatible version of 'pyarrow' installed, please install a version that adheres to: 'pyarrow<0.18.0,>=0.17.0; extra == "pandas"'
   yq: Error running jq: ParserError: while parsing a block mapping
     in "<stdin>", line 1, column 1
   expected <block end>, but found '{'
     in "<stdin>", line 1, column 27.
   ```


----------------------------------------------------------------
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



[GitHub] [airflow] XD-DENG commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r532261262



##########
File path: docs/usage-cli.rst
##########
@@ -174,3 +174,40 @@ You will see a similar result as in the screenshot below.
 .. figure:: img/usage_cli_imgcat.png
 
     Preview of DAG in iTerm2
+
+Formatting commands output
+--------------------------
+
+Some Airflow commands like ``dags list`` or ``tasks states-for-dag-run`` support ``--output`` flag which allow users
+to change the formatting of command's output. Possible options:
+
+  - ``table`` - renders the information as a plain text table
+  - ``json`` - renders the information in form of json string
+  - ``yaml`` - render the information in form of valid yaml
+
+Both ``json`` and ``yaml`` formats make it easier to manipulate the data using command line tools like
+`jq <https://stedolan.github.io/jq/>`__ or `yq <https://kislyuk.github.io/yq/>`__. For example:
+
+.. code-block:: bash
+
+  airflow tasks states-for-dag-run  example_complex 2020-11-13T00:00:00+00:00 --output json | jq ".[] | {sd: .start_date, ed: .end_date}"

Review comment:
       ```suggestion
     airflow tasks states-for-dag-run example_complex 2020-11-13T00:00:00+00:00 --output json | jq ".[] | {sd: .start_date, ed: .end_date}"
   ```

##########
File path: airflow/cli/simple_table.py
##########
@@ -14,11 +14,76 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+import json
+from typing import Any, Callable, Dict, List, Optional, Union
 
+import yaml
 from rich.box import ASCII_DOUBLE_HEAD
+from rich.console import Console
+from rich.syntax import Syntax
 from rich.table import Table
 
 
+class AirflowConsole(Console):
+    """Airflow rich console"""
+
+    def print_as_json(self, data: Dict):
+        """Renders dict as json text representation"""
+        json_content = json.dumps(data)
+        self.print(Syntax(json_content, "json", theme="ansi_dark"), soft_wrap=True)
+
+    def print_as_yaml(self, data: Dict):
+        """Renders dict as yaml text representation"""
+        yaml_content = yaml.dump(data)
+        self.print(Syntax(yaml_content, "yaml", theme="ansi_dark"), soft_wrap=True)
+
+    def print_as_table(self, data: List[Dict]):
+        """Renders list of dictionaries as table"""
+        if not data:
+            self.print("No data found")
+            return
+
+        table = SimpleTable(
+            show_header=True,
+        )
+        for col in data[0].keys():
+            table.add_column(col)
+
+        for row in data:
+            table.add_row(*[str(d) for d in row.values()])
+        self.print(table)
+
+    def _normalize_data(self, value: Any, output: str) -> Union[list, str, dict]:
+        if isinstance(value, (tuple, list)):
+            if output == "table":
+                return ",".join(self._normalize_data(x, output) for x in value)
+            return [self._normalize_data(x, output) for x in value]
+        if isinstance(value, dict) and output != "table":
+            return {k: self._normalize_data(v, output) for k, v in value.items()}
+        return str(value)
+
+    def print_as(self, data: List[Union[Dict, Any]], output: str, mapper: Optional[Callable] = None):
+        """Prints provided using format specified by output argument"""
+        output_to_rendered = {
+            "json": self.print_as_json,
+            "yaml": self.print_as_yaml,
+            "table": self.print_as_table,
+        }
+        renderer = output_to_rendered.get(output)
+        if not renderer:
+            raise ValueError("The output")

Review comment:
       We may need a more meaningful error message here.

##########
File path: airflow/cli/simple_table.py
##########
@@ -14,11 +14,76 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+import json
+from typing import Any, Callable, Dict, List, Optional, Union
 
+import yaml
 from rich.box import ASCII_DOUBLE_HEAD
+from rich.console import Console
+from rich.syntax import Syntax
 from rich.table import Table
 
 
+class AirflowConsole(Console):
+    """Airflow rich console"""
+
+    def print_as_json(self, data: Dict):
+        """Renders dict as json text representation"""
+        json_content = json.dumps(data)
+        self.print(Syntax(json_content, "json", theme="ansi_dark"), soft_wrap=True)
+
+    def print_as_yaml(self, data: Dict):
+        """Renders dict as yaml text representation"""
+        yaml_content = yaml.dump(data)
+        self.print(Syntax(yaml_content, "yaml", theme="ansi_dark"), soft_wrap=True)
+
+    def print_as_table(self, data: List[Dict]):
+        """Renders list of dictionaries as table"""
+        if not data:
+            self.print("No data found")
+            return
+
+        table = SimpleTable(
+            show_header=True,
+        )
+        for col in data[0].keys():
+            table.add_column(col)
+
+        for row in data:
+            table.add_row(*[str(d) for d in row.values()])
+        self.print(table)
+
+    def _normalize_data(self, value: Any, output: str) -> Union[list, str, dict]:
+        if isinstance(value, (tuple, list)):
+            if output == "table":
+                return ",".join(self._normalize_data(x, output) for x in value)
+            return [self._normalize_data(x, output) for x in value]
+        if isinstance(value, dict) and output != "table":
+            return {k: self._normalize_data(v, output) for k, v in value.items()}
+        return str(value)
+
+    def print_as(self, data: List[Union[Dict, Any]], output: str, mapper: Optional[Callable] = None):
+        """Prints provided using format specified by output argument"""
+        output_to_rendered = {

Review comment:
       Maybe you meant `output_to_renderer`?




----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek commented on pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#issuecomment-735413321


   The only thing that I'm wondering about is suppressing / removing log records when using json/yaml. Because otherwise the output is not valid. For example:
   ```
   root@e794bcc2d698:/opt/airflow# airflow dags list --output yaml | yq "."
   /opt/airflow/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py:26 DeprecationWarning: This module is deprecated. Please use `kubernetes.client.models.V1Volume`.
   /opt/airflow/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py:27 DeprecationWarning: This module is deprecated. Please use `kubernetes.client.models.V1VolumeMount`.
   yq: Error running jq: ParserError: expected '<document start>', but found '{'
     in "<stdin>", line 1, column 27.
   [
     "2020-11-29T15:41:59",
     435
   ]
   ```


----------------------------------------------------------------
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



[GitHub] [airflow] XD-DENG commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533708206



##########
File path: UPDATING.md
##########
@@ -52,6 +52,39 @@ assists users migrating to a new version.
 
 ## Master
 
+### Changes to output argument in commands
+
+Instead of using [tabulate](https://pypi.org/project/tabulate/) to render commands output
+we use [rich](https://github.com/willmcgugan/rich). Due to this change the `--output` argument
+will no longer accept formats of tabulate tables. Instead it accepts:
+
+- `table` - will render the output in predefined table
+- `json` - will render the output as a json
+- `yaml` - will render the output as yaml
+
+By doing this we increased consistency and gave users possibility to manipulate the
+output programmatically (when using json or yaml).
+
+Affected commands:
+
+- `airflow dags list`
+- `airflow dags report`
+- `airflow dags list-runs`
+- `airflow dags list-jobs`
+- `airflow connections list`
+- `airflow connections get`
+- `airflow pools list`
+- `airflow pools get`
+- `airflow pools set`
+- `airflow pools delete`
+- `airflow pools export`
+- `airflow role list`
+- `airflow providers list`
+- `airflow providers get`
+- `airflow tasks states-for-dag-run`
+- `airflow users list`
+- `airflow variables list`

Review comment:
       @turbaszek after your latest change (some commands don't print table anymore), maybe we need to further update this doc as well.




----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r532868716



##########
File path: docs/usage-cli.rst
##########
@@ -174,3 +174,40 @@ You will see a similar result as in the screenshot below.
 .. figure:: img/usage_cli_imgcat.png
 
     Preview of DAG in iTerm2
+
+Formatting commands output
+--------------------------
+
+Some Airflow commands like ``dags list`` or ``tasks states-for-dag-run`` support ``--output`` flag which allow users

Review comment:
       Done in caf81ff




----------------------------------------------------------------
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



[GitHub] [airflow] github-actions[bot] commented on pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#issuecomment-735735888


   [The Workflow run](https://github.com/apache/airflow/actions/runs/391446237) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533237284



##########
File path: airflow/cli/commands/pool_command.py
##########
@@ -60,17 +61,20 @@ def pool_get(args):
 def pool_set(args):
     """Creates new pool with a given name and slots"""
     api_client = get_current_api_client()
-    pools = [api_client.create_pool(name=args.pool, slots=args.slots, description=args.description)]
-    _show_pools(pools=pools, output=args.output)
+    api_client.create_pool(name=args.pool, slots=args.slots, description=args.description)
+    print("Pool created")

Review comment:
       I would like to address this in followup PR. Probably create new decorator that will handle known errors and show only message instead of lines of traceback to make it more user friendly. 




----------------------------------------------------------------
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



[GitHub] [airflow] kaxil commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533827933



##########
File path: tests/cli/commands/test_task_command.py
##########
@@ -55,6 +55,7 @@ def reset(dag_id):
         runs.delete()
 
 
+# TODO: Check if tests needs side effects - locally there's missing DAG

Review comment:
       Weird:
   
   ```
   ❯ ./breeze --python 3.6 --backend postgres --db-reset
   root@34129b165f1a:/opt/airflow# pytest tests/cli/commands/test_task_command.py::TestCliTasks
   ==================================================================================================================== test session starts =====================================================================================================================
   platform linux -- Python 3.6.12, pytest-6.1.2, py-1.9.0, pluggy-0.13.1 -- /usr/local/bin/python
   cachedir: .pytest_cache
   rootdir: /opt/airflow, configfile: pytest.ini
   plugins: forked-1.3.0, timeouts-1.2.1, cov-2.10.1, celery-4.4.7, flaky-3.7.0, xdist-2.1.0, instafail-0.4.2, requests-mock-1.8.0, rerunfailures-9.1.1
   setup timeout: 0.0s, execution timeout: 0.0s, teardown timeout: 0.0s
   collected 17 items
   
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_list_tasks PASSED                                                                                                                                                                      [  5%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run PASSED                                                                                                                                                                             [ 11%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_0__ignore_all_dependencies PASSED                                                                                                                               [ 17%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_1__ignore_depends_on_past PASSED                                                                                                                                [ 23%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_2__ignore_dependencies PASSED                                                                                                                                   [ 29%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_3__force PASSED                                                                                                                                                 [ 35%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_mutually_exclusive PASSED                                                                                                                                                          [ 41%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_test PASSED                                                                                                                                                                            [ 47%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_test_with_env_vars PASSED                                                                                                                                                              [ 52%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_test_with_params PASSED                                                                                                                                                                [ 58%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_local_run SKIPPED                                                                                                                                                                          [ 64%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_parentdag_downstream_clear PASSED                                                                                                                                                          [ 70%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_run_naive_taskinstance PASSED                                                                                                                                                              [ 76%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_subdag_clear PASSED                                                                                                                                                                        [ 82%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_task_state PASSED                                                                                                                                                                          [ 88%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_task_states_for_dag_run PASSED                                                                                                                                                             [ 94%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_test PASSED                                                                                                                                                                                [100%]
   
   ====================================================================================================================== warnings summary ======================================================================================================================
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_list_tasks
     /usr/local/lib/python3.6/importlib/_bootstrap.py:219: RuntimeWarning: numpy.ufunc size changed, may indicate binary incompatibility. Expected 192 from C header, got 216 from PyObject
       return f(*args, **kwds)
   
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_list_tasks
     /usr/local/lib/python3.6/site-packages/sqlalchemy/sql/sqltypes.py:271: SAWarning: Unicode type received non-unicode bind param value "b'# -*- coding: utf-8 -*-\\n#'...". (this warning may be suppressed after 10 occurrences)
       (util.ellipses_string(value),),
   
   tests/cli/commands/test_task_command.py::TestCliTasks::test_parentdag_downstream_clear
     /opt/airflow/airflow/models/dag.py:1195: DeprecationWarning: This method is deprecated and will be removed in a future version. Please use partial_subset
       include_downstream=True,
   
   -- Docs: https://docs.pytest.org/en/stable/warnings.html
   ================================================================================================================== short test summary info ===================================================================================================================
   SKIPPED [1] tests/conftest.py:313: The test is skipped because it has quarantined marker. And --include-quarantined flag is passed to pytest. <TestCaseFunction test_local_run>
   =================================================================================================== 16 passed, 1 skipped, 3 warnings in 165.93s (0:02:45) ====================================================================================================
   ```




----------------------------------------------------------------
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



[GitHub] [airflow] github-actions[bot] commented on pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#issuecomment-736494247


   The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.


----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek edited a comment on pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek edited a comment on pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#issuecomment-735413321


   The only thing that I'm wondering about is suppressing / removing log records when using json/yaml. Because otherwise the output is not valid. For example:
   ```
   root@e794bcc2d698:/opt/airflow# airflow dags list --output yaml | yq "."
   /opt/airflow/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py:26 DeprecationWarning: This module is deprecated. Please use `kubernetes.client.models.V1Volume`.
   /opt/airflow/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py:27 DeprecationWarning: This module is deprecated. Please use `kubernetes.client.models.V1VolumeMount`.
   yq: Error running jq: ParserError: expected '<document start>', but found '{'
     in "<stdin>", line 1, column 27.
   [
     "2020-11-29T15:41:59",
     435
   ]
   ```
   or
   ```
   ^[[Aroot@e794bcc2d698:/opt/airflow# airflow providers hooks --output yaml | yq ".[]"
   /usr/local/lib/python3.6/site-packages/snowflake/connector/options.py:39 UserWarning: You have an incompatible version of 'pyarrow' installed, please install a version that adheres to: 'pyarrow<0.18.0,>=0.17.0; extra == "pandas"'
   yq: Error running jq: ParserError: while parsing a block mapping
     in "<stdin>", line 1, column 1
   expected <block end>, but found '{'
     in "<stdin>", line 1, column 27.
   ```
   
   It can also appear inline with the output:
   ```
   Config info
   executor             | LocalExecutor
   task_logging_handler | airflow.utils.log.file_task_handler.FileTaskHandler
   sql_alchemy_conn     | postgresql+psycopg2://postgres:airflow@postgres/airflow
   dags_folder          | /files/dags
   plugins_folder       | /root/airflow/plugins
   base_log_folder      | /root/airflow/logs
   
   [2020-11-29 15:46:57,661] {providers_manager.py:207} WARNING - Exception when importing 'airflow.providers.google.cloud.hooks.compute_ssh.ComputeEngineSSHHook' from 'apache-airflow-providers-google' package: No module named 'google.cloud.oslogin_v1'
   /usr/local/lib/python3.6/site-packages/snowflake/connector/options.py:39 UserWarning: You have an incompatible version of 'pyarrow' installed, please install a version that adheres to: 'pyarrow<0.18.0,>=0.17.0; extra == "pandas"'
   Providers info
   apache-airflow-providers-amazon           | 1.0.0b2
   apache-airflow-providers-apache-cassandra | 1.0.0b2
   apache-airflow-providers-apache-druid     | 1.0.0b2
   ```
   
   I've never seen this in any other application, so I lean toward suppressing warnings and logs.


----------------------------------------------------------------
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



[GitHub] [airflow] github-actions[bot] commented on pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#issuecomment-735670606


   [The Workflow run](https://github.com/apache/airflow/actions/runs/391238580) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
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



[GitHub] [airflow] XD-DENG commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r532877922



##########
File path: airflow/cli/commands/pool_command.py
##########
@@ -21,61 +21,74 @@
 import sys
 from json import JSONDecodeError
 
-from tabulate import tabulate
-
 from airflow.api.client import get_current_api_client
+from airflow.cli.simple_table import AirflowConsole
 from airflow.utils import cli as cli_utils
+from airflow.utils.cli import suppress_logs_and_warning
 
 
-def _tabulate_pools(pools, tablefmt="fancy_grid"):
-    return "\n%s" % tabulate(pools, ['Pool', 'Slots', 'Description'], tablefmt=tablefmt)
+def _show_pools(pools, output):
+    AirflowConsole().print_as(
+        data=pools,
+        output=output,
+        mapper=lambda x: {
+            "pool": x[0],
+            "slots": x[1],
+            "description": x[2],
+        },
+    )
 
 
+@suppress_logs_and_warning()
 def pool_list(args):
     """Displays info of all the pools"""
     api_client = get_current_api_client()
     pools = api_client.get_pools()
-    print(_tabulate_pools(pools=pools, tablefmt=args.output))
+    _show_pools(pools=pools, output=args.output)
 
 
+@suppress_logs_and_warning()
 def pool_get(args):
     """Displays pool info by a given name"""
     api_client = get_current_api_client()
     pools = [api_client.get_pool(name=args.pool)]
-    print(_tabulate_pools(pools=pools, tablefmt=args.output))
+    _show_pools(pools=pools, output=args.output)
 
 
 @cli_utils.action_logging
+@suppress_logs_and_warning()
 def pool_set(args):
     """Creates new pool with a given name and slots"""
     api_client = get_current_api_client()
     pools = [api_client.create_pool(name=args.pool, slots=args.slots, description=args.description)]
-    print(_tabulate_pools(pools=pools, tablefmt=args.output))
+    _show_pools(pools=pools, output=args.output)
 
 
 @cli_utils.action_logging
+@suppress_logs_and_warning()
 def pool_delete(args):
     """Deletes pool by a given name"""
     api_client = get_current_api_client()
     pools = [api_client.delete_pool(name=args.pool)]
-    print(_tabulate_pools(pools=pools, tablefmt=args.output))
+    _show_pools(pools=pools, output=args.output)
 
 
 @cli_utils.action_logging
+@suppress_logs_and_warning()
 def pool_import(args):
     """Imports pools from the file"""
     if not os.path.exists(args.file):
         sys.exit("Missing pools file.")
     pools, failed = pool_import_helper(args.file)
-    print(_tabulate_pools(pools=pools, tablefmt=args.output))
+    _show_pools(pools=pools, output=args.output)
     if len(failed) > 0:
         sys.exit("Failed to update pool(s): {}".format(", ".join(failed)))
 
 
 def pool_export(args):
     """Exports all of the pools to the file"""
     pools = pool_export_helper(args.file)
-    print(_tabulate_pools(pools=pools, tablefmt=args.output))
+    _show_pools(pools=pools, output=args.output)

Review comment:
       Personally I agree with the three points you raised (including this one).




----------------------------------------------------------------
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



[GitHub] [airflow] kaxil commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533748403



##########
File path: tests/cli/commands/test_task_command.py
##########
@@ -55,6 +55,7 @@ def reset(dag_id):
         runs.delete()
 
 
+# TODO: Check if tests needs side effects - locally there's missing DAG

Review comment:
       Can this be removed, looks like tests are passing

##########
File path: tests/cli/commands/test_dag_command.py
##########
@@ -47,6 +47,7 @@
 )
 
 
+# TODO: Check if tests needs side effects - locally there's missing DAG

Review comment:
       Can this be removed, looks like tests are passing




----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533799409



##########
File path: tests/cli/commands/test_task_command.py
##########
@@ -55,6 +55,7 @@ def reset(dag_id):
         runs.delete()
 
 
+# TODO: Check if tests needs side effects - locally there's missing DAG

Review comment:
       > I can test that and fix it if that is the case in a separate PR
   
   Thanks for helping! The problems I observed:
   ```
   root@01658a8a09d3:/opt/airflow# pytest -s tests/cli/commands/test_task_command.py
   ...
   ============================================================================================= short test summary info ==============================================================================================
   SKIPPED [1] tests/conftest.py:313: The test is skipped because it has quarantined marker. And --include-quarantined flag is passed to pytest. <TestCaseFunction test_local_run>
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_list_tasks - airflow.exceptions.AirflowException: dag_id could not be found: example_bash_operator. Either the dag did not exist or it fai...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run - airflow.exceptions.AirflowException: dag_id could not be found: example_bash_operator. Either the dag did not exist or it failed to ...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_0__ignore_all_dependencies - AssertionError: "Option --raw does not work with some of the other options on this com...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_1__ignore_depends_on_past - AssertionError: "Option --raw does not work with some of the other options on this comm...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_2__ignore_dependencies - AssertionError: "Option --raw does not work with some of the other options on this command...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_3__force - AssertionError: "Option --raw does not work with some of the other options on this command." does not ma...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_mutually_exclusive - AssertionError: "Option --raw and --local are mutually exclusive." does not match "dag_id could not be found: exa...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_test - airflow.exceptions.AirflowException: dag_id could not be found: example_bash_operator. Either the dag did not exist or it failed to...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_test_with_env_vars - airflow.exceptions.AirflowException: dag_id could not be found: example_passing_params_via_test_command. Either the d...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_test_with_params - airflow.exceptions.AirflowException: dag_id could not be found: example_passing_params_via_test_command. Either the dag...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_parentdag_downstream_clear - airflow.exceptions.AirflowException: dag_id could not be found: example_subdag_operator.section-1. Either the dag...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_subdag_clear - airflow.exceptions.AirflowException: dag_id could not be found: example_subdag_operator. Either the dag did not exist or it fai...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_task_state - airflow.exceptions.AirflowException: dag_id could not be found: example_bash_operator. Either the dag did not exist or it failed ...
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_task_states_for_dag_run - KeyError: 'example_python_operator'
   FAILED tests/cli/commands/test_task_command.py::TestCliTasks::test_test - airflow.exceptions.AirflowException: dag_id could not be found: example_python_operator. Either the dag did not exist or it failed to p...
   FAILED tests/cli/commands/test_task_command.py::TestLogsfromTaskRunCommand::test_logging_with_run_task - AssertionError: 2 != 1
   =============================================================================== 16 failed, 6 passed, 1 skipped, 1 warning in 41.37s ================================================================================
   ```
   
   This is what I get using brezee and current main branch. 




----------------------------------------------------------------
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



[GitHub] [airflow] kaxil commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533827933



##########
File path: tests/cli/commands/test_task_command.py
##########
@@ -55,6 +55,7 @@ def reset(dag_id):
         runs.delete()
 
 
+# TODO: Check if tests needs side effects - locally there's missing DAG

Review comment:
       Weird:
   
   ```
   ❯ ./breeze --python 3.6 --backend postgres --db-reset
   ===============================================================================================
                Checking integrations and backends
   ===============================================================================================
   PostgreSQL: OK.
   -----------------------------------------------------------------------------------------------
   -----------------------------------------------------------------------------------------------
   
   Resetting the DB
   
   DB: postgresql+psycopg2://postgres:***@postgres/airflow
   [2020-12-02 01:05:00,245] {db.py:619} INFO - Dropping tables that exist
   [2020-12-02 01:05:00,553] {migration.py:155} INFO - Context impl PostgresqlImpl.
   [2020-12-02 01:05:00,554] {migration.py:162} INFO - Will assume transactional DDL.
   [2020-12-02 01:05:00,968] {db.py:609} INFO - Creating tables
   INFO  [alembic.runtime.migration] Context impl PostgresqlImpl.
   INFO  [alembic.runtime.migration] Will assume transactional DDL.
   INFO  [alembic.runtime.migration] Running upgrade  -> e3a246e0dc1, current schema
   INFO  [alembic.runtime.migration] Running upgrade e3a246e0dc1 -> 1507a7289a2f, create is_encrypted
   INFO  [alembic.runtime.migration] Running upgrade 1507a7289a2f -> 13eb55f81627, maintain history for compatibility with earlier migrations
   INFO  [alembic.runtime.migration] Running upgrade 13eb55f81627 -> 338e90f54d61, More logging into task_instance
   INFO  [alembic.runtime.migration] Running upgrade 338e90f54d61 -> 52d714495f0, job_id indices
   INFO  [alembic.runtime.migration] Running upgrade 52d714495f0 -> 502898887f84, Adding extra to Log
   INFO  [alembic.runtime.migration] Running upgrade 502898887f84 -> 1b38cef5b76e, add dagrun
   INFO  [alembic.runtime.migration] Running upgrade 1b38cef5b76e -> 2e541a1dcfed, task_duration
   INFO  [alembic.runtime.migration] Running upgrade 2e541a1dcfed -> 40e67319e3a9, dagrun_config
   INFO  [alembic.runtime.migration] Running upgrade 40e67319e3a9 -> 561833c1c74b, add password column to user
   INFO  [alembic.runtime.migration] Running upgrade 561833c1c74b -> 4446e08588, dagrun start end
   INFO  [alembic.runtime.migration] Running upgrade 4446e08588 -> bbc73705a13e, Add notification_sent column to sla_miss
   INFO  [alembic.runtime.migration] Running upgrade bbc73705a13e -> bba5a7cfc896, Add a column to track the encryption state of the 'Extra' field in connection
   INFO  [alembic.runtime.migration] Running upgrade bba5a7cfc896 -> 1968acfc09e3, add is_encrypted column to variable table
   INFO  [alembic.runtime.migration] Running upgrade 1968acfc09e3 -> 2e82aab8ef20, rename user table
   INFO  [alembic.runtime.migration] Running upgrade 2e82aab8ef20 -> 211e584da130, add TI state index
   INFO  [alembic.runtime.migration] Running upgrade 211e584da130 -> 64de9cddf6c9, add task fails journal table
   INFO  [alembic.runtime.migration] Running upgrade 64de9cddf6c9 -> f2ca10b85618, add dag_stats table
   INFO  [alembic.runtime.migration] Running upgrade f2ca10b85618 -> 4addfa1236f1, Add fractional seconds to mysql tables
   INFO  [alembic.runtime.migration] Running upgrade 4addfa1236f1 -> 8504051e801b, xcom dag task indices
   INFO  [alembic.runtime.migration] Running upgrade 8504051e801b -> 5e7d17757c7a, add pid field to TaskInstance
   INFO  [alembic.runtime.migration] Running upgrade 5e7d17757c7a -> 127d2bf2dfa7, Add dag_id/state index on dag_run table
   INFO  [alembic.runtime.migration] Running upgrade 127d2bf2dfa7 -> cc1e65623dc7, add max tries column to task instance
   INFO  [alembic.runtime.migration] Running upgrade cc1e65623dc7 -> bdaa763e6c56, Make xcom value column a large binary
   INFO  [alembic.runtime.migration] Running upgrade bdaa763e6c56 -> 947454bf1dff, add ti job_id index
   INFO  [alembic.runtime.migration] Running upgrade 947454bf1dff -> d2ae31099d61, Increase text size for MySQL (not relevant for other DBs' text types)
   INFO  [alembic.runtime.migration] Running upgrade d2ae31099d61 -> 0e2a74e0fc9f, Add time zone awareness
   INFO  [alembic.runtime.migration] Running upgrade d2ae31099d61 -> 33ae817a1ff4, kubernetes_resource_checkpointing
   INFO  [alembic.runtime.migration] Running upgrade 33ae817a1ff4 -> 27c6a30d7c24, kubernetes_resource_checkpointing
   INFO  [alembic.runtime.migration] Running upgrade 27c6a30d7c24 -> 86770d1215c0, add kubernetes scheduler uniqueness
   INFO  [alembic.runtime.migration] Running upgrade 86770d1215c0, 0e2a74e0fc9f -> 05f30312d566, merge heads
   INFO  [alembic.runtime.migration] Running upgrade 05f30312d566 -> f23433877c24, fix mysql not null constraint
   INFO  [alembic.runtime.migration] Running upgrade f23433877c24 -> 856955da8476, fix sqlite foreign key
   INFO  [alembic.runtime.migration] Running upgrade 856955da8476 -> 9635ae0956e7, index-faskfail
   INFO  [alembic.runtime.migration] Running upgrade 9635ae0956e7 -> dd25f486b8ea, add idx_log_dag
   INFO  [alembic.runtime.migration] Running upgrade dd25f486b8ea -> bf00311e1990, add index to taskinstance
   INFO  [alembic.runtime.migration] Running upgrade 9635ae0956e7 -> 0a2a5b66e19d, add task_reschedule table
   INFO  [alembic.runtime.migration] Running upgrade 0a2a5b66e19d, bf00311e1990 -> 03bc53e68815, merge_heads_2
   INFO  [alembic.runtime.migration] Running upgrade 03bc53e68815 -> 41f5f12752f8, add superuser field
   INFO  [alembic.runtime.migration] Running upgrade 41f5f12752f8 -> c8ffec048a3b, add fields to dag
   INFO  [alembic.runtime.migration] Running upgrade c8ffec048a3b -> dd4ecb8fbee3, Add schedule interval to dag
   INFO  [alembic.runtime.migration] Running upgrade dd4ecb8fbee3 -> 939bb1e647c8, task reschedule fk on cascade delete
   INFO  [alembic.runtime.migration] Running upgrade 939bb1e647c8 -> 6e96a59344a4, Make TaskInstance.pool not nullable
   INFO  [alembic.runtime.migration] Running upgrade 6e96a59344a4 -> d38e04c12aa2, add serialized_dag table
   INFO  [alembic.runtime.migration] Running upgrade d38e04c12aa2 -> b3b105409875, add root_dag_id to DAG
   INFO  [alembic.runtime.migration] Running upgrade 6e96a59344a4 -> 74effc47d867, change datetime to datetime2(6) on MSSQL tables
   INFO  [alembic.runtime.migration] Running upgrade 939bb1e647c8 -> 004c1210f153, increase queue name size limit
   INFO  [alembic.runtime.migration] Running upgrade c8ffec048a3b -> a56c9515abdc, Remove dag_stat table
   INFO  [alembic.runtime.migration] Running upgrade a56c9515abdc, 004c1210f153, 74effc47d867, b3b105409875 -> 08364691d074, Merge the four heads back together
   INFO  [alembic.runtime.migration] Running upgrade 08364691d074 -> fe461863935f, increase_length_for_connection_password
   INFO  [alembic.runtime.migration] Running upgrade fe461863935f -> 7939bcff74ba, Add DagTags table
   INFO  [alembic.runtime.migration] Running upgrade 7939bcff74ba -> a4c2fd67d16b, add pool_slots field to task_instance
   INFO  [alembic.runtime.migration] Running upgrade a4c2fd67d16b -> 852ae6c715af, Add RenderedTaskInstanceFields table
   INFO  [alembic.runtime.migration] Running upgrade 852ae6c715af -> 952da73b5eff, add dag_code table
   INFO  [alembic.runtime.migration] Running upgrade 952da73b5eff -> a66efa278eea, Add Precision to execution_date in RenderedTaskInstanceFields table
   INFO  [alembic.runtime.migration] Running upgrade a66efa278eea -> cf5dc11e79ad, drop_user_and_chart
   INFO  [alembic.runtime.migration] Running upgrade cf5dc11e79ad -> bbf4a7ad0465, Remove id column from xcom
   INFO  [alembic.runtime.migration] Running upgrade bbf4a7ad0465 -> b25a55525161, Increase length of pool name
   INFO  [alembic.runtime.migration] Running upgrade b25a55525161 -> 3c20cacc0044, Add DagRun run_type
   INFO  [alembic.runtime.migration] Running upgrade 3c20cacc0044 -> 8f966b9c467a, Set conn_type as non-nullable
   INFO  [alembic.runtime.migration] Running upgrade 8f966b9c467a -> 8d48763f6d53, add unique constraint to conn_id
   INFO  [alembic.runtime.migration] Running upgrade 8d48763f6d53 -> da3f683c3a5a, Add dag_hash Column to serialized_dag table
   INFO  [alembic.runtime.migration] Running upgrade da3f683c3a5a -> 92c57b58940d, Create FAB Tables
   INFO  [alembic.runtime.migration] Running upgrade 92c57b58940d -> 03afc6b6f902, Increase length of FAB ab_view_menu.name column
   INFO  [alembic.runtime.migration] Running upgrade 03afc6b6f902 -> e38be357a868, Add sensor_instance table
   INFO  [alembic.runtime.migration] Running upgrade e38be357a868 -> b247b1e3d1ed, Add queued by Job ID to TI
   INFO  [alembic.runtime.migration] Running upgrade b247b1e3d1ed -> e1a11ece99cc, Add external executor ID to TI
   INFO  [alembic.runtime.migration] Running upgrade e1a11ece99cc -> bef4f3d11e8b, Drop KubeResourceVersion and KubeWorkerId
   INFO  [alembic.runtime.migration] Running upgrade bef4f3d11e8b -> 98271e7606e2, Add scheduling_decision to DagRun and DAG
   INFO  [alembic.runtime.migration] Running upgrade 98271e7606e2 -> 52d53670a240, fix_mssql_exec_date_rendered_task_instance_fields_for_MSSQL
   INFO  [alembic.runtime.migration] Running upgrade 52d53670a240 -> 364159666cbd, Add creating_job_id to DagRun table
   INFO  [alembic.runtime.migration] Running upgrade 364159666cbd -> 45ba3f1493b9, add-k8s-yaml-to-rendered-templates
   INFO  [alembic.runtime.migration] Running upgrade 45ba3f1493b9 -> 849da589634d, Prefix DAG permissions.
   [2020-12-02 01:05:05,538] {manager.py:727} WARNING - No user yet created, use flask fab command to do it.
   root@34129b165f1a:/opt/airflow# pytest tests/cli/commands/test_task_command.py::TestCliTasks
   ==================================================================================================================== test session starts =====================================================================================================================
   platform linux -- Python 3.6.12, pytest-6.1.2, py-1.9.0, pluggy-0.13.1 -- /usr/local/bin/python
   cachedir: .pytest_cache
   rootdir: /opt/airflow, configfile: pytest.ini
   plugins: forked-1.3.0, timeouts-1.2.1, cov-2.10.1, celery-4.4.7, flaky-3.7.0, xdist-2.1.0, instafail-0.4.2, requests-mock-1.8.0, rerunfailures-9.1.1
   setup timeout: 0.0s, execution timeout: 0.0s, teardown timeout: 0.0s
   collected 17 items
   
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_list_tasks PASSED                                                                                                                                                                      [  5%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run PASSED                                                                                                                                                                             [ 11%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_0__ignore_all_dependencies PASSED                                                                                                                               [ 17%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_1__ignore_depends_on_past PASSED                                                                                                                                [ 23%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_2__ignore_dependencies PASSED                                                                                                                                   [ 29%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_invalid_raw_option_3__force PASSED                                                                                                                                                 [ 35%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_run_mutually_exclusive PASSED                                                                                                                                                          [ 41%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_test PASSED                                                                                                                                                                            [ 47%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_test_with_env_vars PASSED                                                                                                                                                              [ 52%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_test_with_params PASSED                                                                                                                                                                [ 58%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_local_run SKIPPED                                                                                                                                                                          [ 64%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_parentdag_downstream_clear PASSED                                                                                                                                                          [ 70%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_run_naive_taskinstance PASSED                                                                                                                                                              [ 76%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_subdag_clear PASSED                                                                                                                                                                        [ 82%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_task_state PASSED                                                                                                                                                                          [ 88%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_task_states_for_dag_run PASSED                                                                                                                                                             [ 94%]
   tests/cli/commands/test_task_command.py::TestCliTasks::test_test PASSED                                                                                                                                                                                [100%]
   
   ====================================================================================================================== warnings summary ======================================================================================================================
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_list_tasks
     /usr/local/lib/python3.6/importlib/_bootstrap.py:219: RuntimeWarning: numpy.ufunc size changed, may indicate binary incompatibility. Expected 192 from C header, got 216 from PyObject
       return f(*args, **kwds)
   
   tests/cli/commands/test_task_command.py::TestCliTasks::test_cli_list_tasks
     /usr/local/lib/python3.6/site-packages/sqlalchemy/sql/sqltypes.py:271: SAWarning: Unicode type received non-unicode bind param value "b'# -*- coding: utf-8 -*-\\n#'...". (this warning may be suppressed after 10 occurrences)
       (util.ellipses_string(value),),
   
   tests/cli/commands/test_task_command.py::TestCliTasks::test_parentdag_downstream_clear
     /opt/airflow/airflow/models/dag.py:1195: DeprecationWarning: This method is deprecated and will be removed in a future version. Please use partial_subset
       include_downstream=True,
   
   -- Docs: https://docs.pytest.org/en/stable/warnings.html
   ================================================================================================================== short test summary info ===================================================================================================================
   SKIPPED [1] tests/conftest.py:313: The test is skipped because it has quarantined marker. And --include-quarantined flag is passed to pytest. <TestCaseFunction test_local_run>
   =================================================================================================== 16 passed, 1 skipped, 3 warnings in 165.93s (0:02:45) ====================================================================================================
   ```




----------------------------------------------------------------
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



[GitHub] [airflow] kaxil commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533746027



##########
File path: UPDATING.md
##########
@@ -52,6 +52,41 @@ assists users migrating to a new version.
 
 ## Master
 
+### Changes to output argument in commands
+
+Instead of using [tabulate](https://pypi.org/project/tabulate/) to render commands output
+we use [rich](https://github.com/willmcgugan/rich). Due to this change the `--output` argument
+will no longer accept formats of tabulate tables. Instead it accepts:

Review comment:
       ```suggestion
   From Airflow 2.0, We are replacing [tabulate](https://pypi.org/project/tabulate/) with [rich](https://github.com/willmcgugan/rich) to render commands output. Due to this change, the `--output` argument
   will no longer accept formats of tabulate tables. Instead, it now accepts:
   ```




----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533789920



##########
File path: tests/cli/commands/test_task_command.py
##########
@@ -55,6 +55,7 @@ def reset(dag_id):
         runs.delete()
 
 
+# TODO: Check if tests needs side effects - locally there's missing DAG

Review comment:
       That's the problem - they don't work locally when triggered without whole test suite 




----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r532262483



##########
File path: airflow/cli/simple_table.py
##########
@@ -14,11 +14,76 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+import json
+from typing import Any, Callable, Dict, List, Optional, Union
 
+import yaml
 from rich.box import ASCII_DOUBLE_HEAD
+from rich.console import Console
+from rich.syntax import Syntax
 from rich.table import Table
 
 
+class AirflowConsole(Console):
+    """Airflow rich console"""
+
+    def print_as_json(self, data: Dict):
+        """Renders dict as json text representation"""
+        json_content = json.dumps(data)
+        self.print(Syntax(json_content, "json", theme="ansi_dark"), soft_wrap=True)
+
+    def print_as_yaml(self, data: Dict):
+        """Renders dict as yaml text representation"""
+        yaml_content = yaml.dump(data)
+        self.print(Syntax(yaml_content, "yaml", theme="ansi_dark"), soft_wrap=True)
+
+    def print_as_table(self, data: List[Dict]):
+        """Renders list of dictionaries as table"""
+        if not data:
+            self.print("No data found")
+            return
+
+        table = SimpleTable(
+            show_header=True,
+        )
+        for col in data[0].keys():
+            table.add_column(col)
+
+        for row in data:
+            table.add_row(*[str(d) for d in row.values()])
+        self.print(table)
+
+    def _normalize_data(self, value: Any, output: str) -> Union[list, str, dict]:
+        if isinstance(value, (tuple, list)):
+            if output == "table":
+                return ",".join(self._normalize_data(x, output) for x in value)
+            return [self._normalize_data(x, output) for x in value]
+        if isinstance(value, dict) and output != "table":
+            return {k: self._normalize_data(v, output) for k, v in value.items()}
+        return str(value)
+
+    def print_as(self, data: List[Union[Dict, Any]], output: str, mapper: Optional[Callable] = None):
+        """Prints provided using format specified by output argument"""
+        output_to_rendered = {
+            "json": self.print_as_json,
+            "yaml": self.print_as_yaml,
+            "table": self.print_as_table,
+        }
+        renderer = output_to_rendered.get(output)
+        if not renderer:
+            raise ValueError("The output")

Review comment:
       Oh, yeah, definitely xD




----------------------------------------------------------------
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



[GitHub] [airflow] XD-DENG commented on pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#issuecomment-735425898


   > The only thing that I'm wondering about is suppressing / removing log records when using json/yaml. Because otherwise the output is not valid. For example:
   > 
   > ... ...
   > I've never seen this in any other application, so I lean toward suppressing warnings and logs.
   
   The same preference for me. 
   
   For `table` output I think we should suppress log as well. not only for json/yaml.
   


----------------------------------------------------------------
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



[GitHub] [airflow] XD-DENG commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r532818688



##########
File path: airflow/cli/simple_table.py
##########
@@ -14,11 +14,78 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+import json
+from typing import Any, Callable, Dict, List, Optional, Union
 
+import yaml
 from rich.box import ASCII_DOUBLE_HEAD
+from rich.console import Console
+from rich.syntax import Syntax
 from rich.table import Table
 
 
+class AirflowConsole(Console):
+    """Airflow rich console"""
+
+    def print_as_json(self, data: Dict):
+        """Renders dict as json text representation"""
+        json_content = json.dumps(data)
+        self.print(Syntax(json_content, "json", theme="ansi_dark"), soft_wrap=True)
+
+    def print_as_yaml(self, data: Dict):
+        """Renders dict as yaml text representation"""
+        yaml_content = yaml.dump(data)
+        self.print(Syntax(yaml_content, "yaml", theme="ansi_dark"), soft_wrap=True)
+
+    def print_as_table(self, data: List[Dict]):
+        """Renders list of dictionaries as table"""
+        if not data:
+            self.print("No data found")
+            return
+
+        table = SimpleTable(
+            show_header=True,
+        )
+        for col in data[0].keys():
+            table.add_column(col)
+
+        for row in data:
+            table.add_row(*[str(d) for d in row.values()])
+        self.print(table)
+
+    def _normalize_data(self, value: Any, output: str) -> Union[list, str, dict]:
+        if isinstance(value, (tuple, list)):
+            if output == "table":
+                return ",".join(self._normalize_data(x, output) for x in value)
+            return [self._normalize_data(x, output) for x in value]
+        if isinstance(value, dict) and output != "table":
+            return {k: self._normalize_data(v, output) for k, v in value.items()}
+        return str(value)
+
+    def print_as(self, data: List[Union[Dict, Any]], output: str, mapper: Optional[Callable] = None):
+        """Prints provided using format specified by output argument"""
+        output_to_renderer = {
+            "json": self.print_as_json,
+            "yaml": self.print_as_yaml,
+            "table": self.print_as_table,
+        }
+        renderer = output_to_renderer.get(output)
+        if not renderer:
+            raise ValueError(
+                f"Unknown formatter: {output}. Allowed options: {list(output_to_renderer.keys())}"
+            )
+
+        if not all(isinstance(d, dict) for d in data) and not mapper:
+            raise ValueError("To tabulate non-dictionary data you need to provider `mapper` function")

Review comment:
       ```suggestion
               raise ValueError("To tabulate non-dictionary data you need to provide `mapper` function")
   ```

##########
File path: docs/usage-cli.rst
##########
@@ -174,3 +174,40 @@ You will see a similar result as in the screenshot below.
 .. figure:: img/usage_cli_imgcat.png
 
     Preview of DAG in iTerm2
+
+Formatting commands output
+--------------------------
+
+Some Airflow commands like ``dags list`` or ``tasks states-for-dag-run`` support ``--output`` flag which allow users

Review comment:
       ```suggestion
   Some Airflow commands like ``airflow dags list`` or ``airflow tasks states-for-dag-run`` support ``--output`` flag which allow users
   ```
   
   This is really minor difference. But I'm trying to think from a 100% newbie perspective: "here is the command, I can simply copy and paste. Oops, why it doesn't work".

##########
File path: UPDATING.md
##########
@@ -52,6 +52,39 @@ assists users migrating to a new version.
 
 ## Master
 
+### Changes to output argument in commands
+
+Instead of using [tabulate](https://pypi.org/project/tabulate/) to render commands output
+we use [rich](https://github.com/willmcgugan/rich). Due to this change the `--output` argument
+will no longer accept formats of tabulate tables. Instead it accepts:
+
+- `table` - will render the output in predefined table
+- `json` - will render the output as a json
+- `yaml` - will render the output as yaml
+
+By doing this we increased consistency and gave users possibility to manipulate the
+output programmatically (when using json or yaml).
+
+Affected commands:
+
+- `airflow dags list`
+- `airflow dags report`
+- `airflow dags list-runs`
+- `airflow dags list-jobs`
+- `airflow connections list`
+- `airflow connections get`
+- `airflow pools list`
+- `airflow pools get`
+- `airflow pools set`
+- `airflow pools delete`
+- `airflow pools export`
+- `airflow role list`
+- `airflow providers list`
+- `airflow providers get`
+- `airflow tasks states-for-dag-run`
+- `airflow users list`
+- `airflow variables list`

Review comment:
       `airflow pools import` and `airflow providers hooks` should be in this list as well?




----------------------------------------------------------------
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



[GitHub] [airflow] github-actions[bot] commented on pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#issuecomment-736464818


   [The Workflow run](https://github.com/apache/airflow/actions/runs/393696204) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533790668



##########
File path: tests/cli/commands/test_dag_command.py
##########
@@ -47,6 +47,7 @@
 )
 
 
+# TODO: Check if tests needs side effects - locally there's missing DAG

Review comment:
       See https://github.com/apache/airflow/pull/12704#discussion_r533789920




----------------------------------------------------------------
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



[GitHub] [airflow] XD-DENG commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r532859799



##########
File path: airflow/cli/commands/connection_command.py
##########
@@ -19,78 +19,54 @@
 import json
 import os
 import sys
-from typing import List
+from typing import Any, Dict, List
 from urllib.parse import urlparse, urlunparse
 
-import pygments
 import yaml
-from pygments.lexers.data import YamlLexer
 from sqlalchemy.orm import exc
-from tabulate import tabulate
 
+from airflow.cli.simple_table import AirflowConsole
 from airflow.exceptions import AirflowNotFoundException
 from airflow.hooks.base_hook import BaseHook
 from airflow.models import Connection
 from airflow.utils import cli as cli_utils
-from airflow.utils.cli import should_use_colors
-from airflow.utils.code_utils import get_terminal_formatter
+from airflow.utils.cli import suppress_logs_and_warning
 from airflow.utils.session import create_session
 
 
-def _tabulate_connection(conns: List[Connection], tablefmt: str):
-    tabulate_data = [
-        {
-            'Conn Id': conn.conn_id,
-            'Conn Type': conn.conn_type,
-            'Description': conn.description,
-            'Host': conn.host,
-            'Port': conn.port,
-            'Is Encrypted': conn.is_encrypted,
-            'Is Extra Encrypted': conn.is_encrypted,
-            'Extra': conn.extra,
-        }
-        for conn in conns
-    ]
-
-    msg = tabulate(tabulate_data, tablefmt=tablefmt, headers='keys')
-    return msg
-
-
-def _yamulate_connection(conn: Connection):
-    yaml_data = {
-        'Id': conn.id,
-        'Conn Id': conn.conn_id,
-        'Conn Type': conn.conn_type,
-        'Description': conn.description,
-        'Host': conn.host,
-        'Schema': conn.schema,
-        'Login': conn.login,
-        'Password': conn.password,
-        'Port': conn.port,
-        'Is Encrypted': conn.is_encrypted,
-        'Is Extra Encrypted': conn.is_encrypted,
-        'Extra': conn.extra_dejson,
-        'URI': conn.get_uri(),
+def _connection_mapper(conn: Connection) -> Dict[str, Any]:
+    return {
+        'id': conn.id,
+        'conn_id': conn.conn_id,
+        'conn_type': conn.conn_type,
+        'description': conn.description,
+        'host': conn.host,
+        'schema': conn.schema,
+        'login': conn.login,
+        'password': conn.password,
+        'port': conn.port,
+        'is_encrypted': conn.is_encrypted,
+        'is_extra_encrypted': conn.is_encrypted,
+        'extra_dejson': conn.extra_dejson,
+        'get_uri()': conn.get_uri(),

Review comment:
       We may not want `get_uri()` as a column name in the table, or an attribute name in yaml/json.




----------------------------------------------------------------
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



[GitHub] [airflow] github-actions[bot] commented on pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#issuecomment-736465787


   [The Workflow run](https://github.com/apache/airflow/actions/runs/393702476) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.


----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533789920



##########
File path: tests/cli/commands/test_task_command.py
##########
@@ -55,6 +55,7 @@ def reset(dag_id):
         runs.delete()
 
 
+# TODO: Check if tests needs side effects - locally there's missing DAG

Review comment:
       That's the problem - they don't work locally when triggered without whole test suite. I plan to create an issue once this get merged so I can reference those comments. 




----------------------------------------------------------------
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



[GitHub] [airflow] kaxil commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533795846



##########
File path: tests/cli/commands/test_task_command.py
##########
@@ -55,6 +55,7 @@ def reset(dag_id):
         runs.delete()
 
 
+# TODO: Check if tests needs side effects - locally there's missing DAG

Review comment:
       I can test that and fix it if that is the case in a separate 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



[GitHub] [airflow] XD-DENG commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r532869104



##########
File path: airflow/cli/commands/task_command.py
##########
@@ -304,43 +309,38 @@ def _guess_debugger():
 
 
 @cli_utils.action_logging
+@suppress_logs_and_warning()
 def task_states_for_dag_run(args):
     """Get the status of all task instances in a DagRun"""
-    session = settings.Session()
-
-    tis = (
-        session.query(
-            TaskInstance.dag_id,
-            TaskInstance.execution_date,
-            TaskInstance.task_id,
-            TaskInstance.state,
-            TaskInstance.start_date,
-            TaskInstance.end_date,
-        )
-        .filter(TaskInstance.dag_id == args.dag_id, TaskInstance.execution_date == args.execution_date)
-        .all()
-    )
-
-    if len(tis) == 0:
-        raise AirflowException("DagRun does not exist.")
-
-    formatted_rows = []
-
-    for ti in tis:
-        formatted_rows.append(
-            (ti.dag_id, ti.execution_date, ti.task_id, ti.state, ti.start_date, ti.end_date)
+    with create_session() as session:
+        tis = (
+            session.query(
+                TaskInstance.dag_id,
+                TaskInstance.execution_date,
+                TaskInstance.task_id,
+                TaskInstance.state,
+                TaskInstance.start_date,
+                TaskInstance.end_date,
+            )
+            .filter(TaskInstance.dag_id == args.dag_id, TaskInstance.execution_date == args.execution_date)
+            .all()
         )
 
-    print(
-        "\n%s"
-        % tabulate(
-            formatted_rows,
-            ['dag', 'exec_date', 'task', 'state', 'start_date', 'end_date'],
-            tablefmt=args.output,
+        if len(tis) == 0:
+            raise AirflowException("DagRun does not exist.")
+
+        AirflowConsole().print_as(
+            data=tis,
+            output=args.output,
+            mapper=lambda ti: {
+                "dag_id": ti.dag_id,
+                "execution_date": ti.execution_date.isoformat(),
+                "task_id": ti.task_id,
+                "state": ti.state,
+                "start_date": ti.start_date.isoformat(),
+                "end_date": ti.end_date.isoformat(),

Review comment:
       Should guards against `None` be added? Similar to what you have in `dag_list_dag_runs()`, i.e.
   
   ```python
                   ... ...
                   "start_date": ti.start_date.isoformat() if ti.start_date else '',
                   "end_date": ti.end_date.isoformat() if ti.end_date else '',
                  ... ....
   ```




----------------------------------------------------------------
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



[GitHub] [airflow] XD-DENG commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r533107633



##########
File path: airflow/cli/commands/pool_command.py
##########
@@ -60,17 +61,20 @@ def pool_get(args):
 def pool_set(args):
     """Creates new pool with a given name and slots"""
     api_client = get_current_api_client()
-    pools = [api_client.create_pool(name=args.pool, slots=args.slots, description=args.description)]
-    _show_pools(pools=pools, output=args.output)
+    api_client.create_pool(name=args.pool, slots=args.slots, description=args.description)
+    print("Pool created")

Review comment:
       Maybe add `try-except` similar to what you added in `pool_delete`?




----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek merged pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek merged pull request #12704:
URL: https://github.com/apache/airflow/pull/12704


   


----------------------------------------------------------------
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



[GitHub] [airflow] turbaszek commented on a change in pull request #12704: Refactor list rendering in commands

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #12704:
URL: https://github.com/apache/airflow/pull/12704#discussion_r532871073



##########
File path: airflow/cli/commands/task_command.py
##########
@@ -304,43 +309,38 @@ def _guess_debugger():
 
 
 @cli_utils.action_logging
+@suppress_logs_and_warning()
 def task_states_for_dag_run(args):
     """Get the status of all task instances in a DagRun"""
-    session = settings.Session()
-
-    tis = (
-        session.query(
-            TaskInstance.dag_id,
-            TaskInstance.execution_date,
-            TaskInstance.task_id,
-            TaskInstance.state,
-            TaskInstance.start_date,
-            TaskInstance.end_date,
-        )
-        .filter(TaskInstance.dag_id == args.dag_id, TaskInstance.execution_date == args.execution_date)
-        .all()
-    )
-
-    if len(tis) == 0:
-        raise AirflowException("DagRun does not exist.")
-
-    formatted_rows = []
-
-    for ti in tis:
-        formatted_rows.append(
-            (ti.dag_id, ti.execution_date, ti.task_id, ti.state, ti.start_date, ti.end_date)
+    with create_session() as session:
+        tis = (
+            session.query(
+                TaskInstance.dag_id,
+                TaskInstance.execution_date,
+                TaskInstance.task_id,
+                TaskInstance.state,
+                TaskInstance.start_date,
+                TaskInstance.end_date,
+            )
+            .filter(TaskInstance.dag_id == args.dag_id, TaskInstance.execution_date == args.execution_date)
+            .all()
         )
 
-    print(
-        "\n%s"
-        % tabulate(
-            formatted_rows,
-            ['dag', 'exec_date', 'task', 'state', 'start_date', 'end_date'],
-            tablefmt=args.output,
+        if len(tis) == 0:
+            raise AirflowException("DagRun does not exist.")
+
+        AirflowConsole().print_as(
+            data=tis,
+            output=args.output,
+            mapper=lambda ti: {
+                "dag_id": ti.dag_id,
+                "execution_date": ti.execution_date.isoformat(),
+                "task_id": ti.task_id,
+                "state": ti.state,
+                "start_date": ti.start_date.isoformat(),
+                "end_date": ti.end_date.isoformat(),

Review comment:
       Hm, usually ti should had both, unless someone is calling this during dag run. Will add it just for safety
   




----------------------------------------------------------------
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