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 2021/03/04 16:51:22 UTC
[GitHub] [airflow] ecerulm opened a new pull request #14607: Do not suppress error in airflow plugins cli
ecerulm opened a new pull request #14607:
URL: https://github.com/apache/airflow/pull/14607
Currently `airflow plugins` with suppress all errors, and warnings, so for example if the plugins contain errors, the output will simply show
```
airflow plugins
No plugins loaded
```
There is the flag `--verbose` that will preserve the log output:
```
airflow plugins --verbose
[2021-03-04 17:46:38,569] {plugins_manager.py:243} ERROR - No module named 'nonexisting'
Traceback (most recent call last):
File "/Users/ecerulm/git/airflow/airflow/plugins_manager.py", line 233, in load_plugins_from_plugin_directory
loader.exec_module(mod)
File "<frozen importlib._bootstrap_external>", line 783, in exec_module
File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
File "/Users/ecerulm/git/airflow/plugins/errored_plugin.py", line 13, in <module>
import nonexisting
ModuleNotFoundError: No module named 'nonexisting'
[2021-03-04 17:46:38,570] {plugins_manager.py:244} ERROR - Failed to import plugin /Users/ecerulm/git/airflow/plugins/errored_plugin.py
No plugins loaded
```
This PR aims to change the default suppression level so that errors are shown by default on `airflow plugins` by introducing a parameter to the `@suppress_logs_and_warnings` decorator to set the supression level.
so `@suppress_log_and_warnings()` will supress all logs but `@suppress_log_and_warnings(level=logging.WARNING)` will still show `ERROR` and `CRITICAL` 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] ecerulm commented on pull request #14607: Do not suppress error in airflow plugins cli
Posted by GitBox <gi...@apache.org>.
ecerulm commented on pull request #14607:
URL: https://github.com/apache/airflow/pull/14607#issuecomment-799214227
@ecerulm , I think the user story goes more like this
1. They run `airflow plugins` and they don't see any errors, they see just `No plugins loaded` or a list of loaded plugins without the plugin they are looking for
2. They don't know that they should use `--verbose` to see errors, because in every other piece of software that they use error are always printed, and they associate `--verbose` with "enable debug logs" not with "put this if you want to see any errors".
I guess it's a matter of taste, I'm confused by seeing just "No plugins loaded", to me it meant I must have put the file in the wrong directory because it's not even trying to load it. I think the experience for users trying to write or install their first plugin would be confusing.
>The main reason behind introducing this errors suppression (in general) was to make the command more clearer and easier to parse using tools like jq (by using -o json).
`airflow plugins -o json` output is `No plugins loaded` which is not valid JSON, should I report that as an issue?
If you want to keep the current behaviour (no logs whatsoever, unless `--verbose`) then I suggest any of these other changes (I can PR if you want) :
* Change the `No plugins loaded` to `No plugins loaded (3 failed loading, run with --verbose to see the actual errors)`. Embed this in each output format
* Disable logs only if output != plain
----------------------------------------------------------------
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] ecerulm commented on pull request #14607: Do not suppress error in airflow plugins cli
Posted by GitBox <gi...@apache.org>.
ecerulm commented on pull request #14607:
URL: https://github.com/apache/airflow/pull/14607#issuecomment-800864584
Then I guess I'd better close this PR and I'll do another with the `No plugins loaded (3 failed loading, run with --verbose to see the actual errors)`
----------------------------------------------------------------
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 #14607: Do not suppress error in airflow plugins cli
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #14607:
URL: https://github.com/apache/airflow/pull/14607#issuecomment-800603880
> * Change the `No plugins loaded` to `No plugins loaded (3 failed loading, run with --verbose to see the actual errors)`. Embed this in each output format
I think this is great proposition! It would be awesome if you would like to add this functionality 🚀
----------------------------------------------------------------
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 #14607: Do not suppress error in airflow plugins cli
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14607:
URL: https://github.com/apache/airflow/pull/14607#issuecomment-790841751
[The Workflow run](https://github.com/apache/airflow/actions/runs/621890607) 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] ecerulm closed pull request #14607: Do not suppress error in airflow plugins cli
Posted by GitBox <gi...@apache.org>.
ecerulm closed pull request #14607:
URL: https://github.com/apache/airflow/pull/14607
----------------------------------------------------------------
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 #14607: Do not suppress error in airflow plugins cli
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #14607:
URL: https://github.com/apache/airflow/pull/14607#issuecomment-798787225
@ecerulm can you explain what's wrong with current solution? The users story I think is as follows:
1. They run `airflow plugins` - they don't see something they expected
2. They add `--verbose` flag so they can learn more
The main reason behind introducing this errors suppression (in general) was to make the command more clearer and easier to parse using tools like jq (by using `-o 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