XD-DENG commented on a change in pull request #13036:
URL: https://github.com/apache/airflow/pull/13036#discussion_r541785722
##########
File path: airflow/cli/simple_table.py
##########
@@ -53,13 +56,18 @@ def print_as_table(self, data: List[Dict]):
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]:
+ # pylint: disable=too-many-return-statements
+ def _normalize_data(self, value: Any, output: str) -> Optional[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()}
+ if isinstance(value, PluginsDirectorySource):
+ return str(value)
+ if inspect.isclass(value):
+ return value.__name__
Review comment:
I understand that we have these two `IF`s in order here because
`PluginsDirectorySource ` is a special `class` to handle (we want to use its
`__str__` rather than `__name__`). But the first glance at it is a bit
confusing because what it returns _looks the same_ to the last return line.
This may be a confusion to people who read this later.
So I would like to suggest changing it to
```python
if inspect.isclass(value) and not isinstance(value, PluginsDirectorySource):
return value.__name__
```
##########
File path: airflow/cli/commands/plugins_command.py
##########
@@ -78,33 +75,17 @@ def dump_plugins(args):
print("No plugins loaded")
return
- console = Console()
- console.print("[bold yellow]SUMMARY:[/bold yellow]")
- console.print(
- f"[bold green]Plugins directory[/bold green]: {conf.get('core',
'plugins_folder')}\n", highlight=False
- )
- console.print(
- f"[bold green]Loaded plugins[/bold green]:
{len(plugins_manager.plugins)}\n", highlight=False
- )
+ plugins_info: List[Dict[str, str]] = []
+ for plugin in plugins_manager.plugins:
+ info = {"name": plugin.name}
+ info.update({n: getattr(plugin, n) for n in
PLUGINS_ATTRIBUTES_TO_DUMP})
+ plugins_info.append(info)
- for attr_name in PLUGINS_MANAGER_ATTRIBUTES_TO_DUMP:
- attr_value: Optional[List[Any]] = getattr(plugins_manager, attr_name)
- if not attr_value:
- continue
- table = SimpleTable(title=attr_name.capitalize().replace("_", " "))
- table.add_column(width=100)
- for item in attr_value: # pylint: disable=not-an-iterable
- table.add_row(
- f"- {_get_name(item)}",
- )
- console.print(table)
+ # Remove empty info
+ if args.output == "table": # pylint: disable=too-many-nested-blocks
+ for col in list(plugins_info[0].keys()): # it will exist as there's
at least one plugin
Review comment:
```suggestion
for col in plugins_info[0]: # it will exist as there's at least one
plugin
```
----------------------------------------------------------------
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:
[email protected]
