Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed deprecation message for "variables" command #14457

Merged

Conversation

brunobastosg
Copy link

closes: #13091

@boring-cyborg
Copy link

boring-cyborg bot commented Feb 25, 2021

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:

  • Pay attention to the quality of your code (flake8, pylint and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@turbaszek turbaszek requested a review from kaxil February 25, 2021 13:01
@@ -414,7 +414,7 @@ def variables_export(args):
_vars_wrapper(args, export=args.file)


@cli_utils.deprecated_action(new_name='variables')
@cli_utils.deprecated_action(new_name='variables list')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not make sense to me:

root@99624d92186f:/opt/airflow# airflow variables
The 'variables' command is deprecated and removed in Airflow 2.0, please use 'variables list' instead

root@99624d92186f:/opt/airflow# airflow variables list
usage: airflow variables [-h] COMMAND ...

Manage variables

positional arguments:
  COMMAND
    delete    Delete variable
    export    Export all variables
    get       Get variable
    import    Import variables
    set       Set variable

optional arguments:
  -h, --help  show this help message and exit

airflow variables command error: argument COMMAND: invalid choice: 'list' (choose from 'delete', 'export', 'get', 'import', 'set'), see help above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kaxil , that's because variables list is missed in the new Variable Commands in 1.10.14 (https://github.com/apache/airflow/blob/1.10.14/airflow/bin/cli.py#L3066), but it is a valid new command in 2.0 (https://github.com/apache/airflow/blob/2.0.1/airflow/cli/cli_parser.py#L1037)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In such a case, we will also need to make further change to ensure variables list present in 1.10.x (say 1.10.15) as well

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old style:

airflow variables -l

New

airflow variables list etc.

Copy link
Member

@XD-DENG XD-DENG Feb 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will shortly prepare a PR against branch v1-10-stable to add airflow variables list for 1.10.15.

Meanwhile, I think this PR is ok to be merged.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prepared PR #14462 to supplement this, as mentioned above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, approved #14462

@github-actions
Copy link

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.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Feb 25, 2021
@XD-DENG XD-DENG added this to the Airflow 1.10.15 milestone Feb 25, 2021
@XD-DENG XD-DENG merged commit 75b2fa2 into apache:v1-10-stable Feb 26, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 26, 2021

Awesome work, congrats on your first merged pull request!

@brunobastosg brunobastosg deleted the 13091-fix-deprecation-message branch February 26, 2021 12:06
linuxfft added a commit to linuxfft/airflow that referenced this pull request Aug 13, 2021
Apache Airflow 1.10.15

- Fix `airflow db upgrade` to upgrade db as intended (apache#13267)
- Moved boto3 limitation to snowflake (apache#13286)
- `KubernetesExecutor` should accept images from `executor_config` (apache#13074)
- Scheduler should acknowledge active runs properly (apache#13803)
- Bugfix: Unable to import Airflow plugins on Python 3.8 (apache#12859)
- Include `airflow/contrib/executors` in the dist package
- Pin Click version for Python 2.7 users
- Ensure all statsd timers use millisecond values. (apache#10633)
- [`kubernetes_generate_dag_yaml`] - Fix dag yaml generate function (apache#13816)
- Fix `airflow tasks clear` cli command wirh `--yes` (apache#14188)
- Fix permission error on non-POSIX filesystem (apache#13121) (apache#14383)
- Fixed deprecation message for "variables" command (apache#14457)
- BugFix: fix the `delete_dag` function of json_client (apache#14441)
- Fix merging of secrets and configmaps for `KubernetesExecutor` (apache#14090)
- Fix webserver exiting when gunicorn master crashes (apache#13470)
- Bump ini from 1.3.5 to 1.3.8 in `airflow/www_rbac`
- Bump datatables.net from 1.10.21 to 1.10.23 in `airflow/www_rbac`
- Webserver: Sanitize string passed to origin param (apache#14738)
- Make `rbac_app`'s `db.session` use the same timezone with `@provide_session` (apache#14025)

- Adds airflow as viable docker command in official image (apache#12878)
- `StreamLogWriter`: Provide (no-op) close method (apache#10885)
- Add 'airflow variables list' command for 1.10.x transition version (apache#14462)

- Update URL for Airflow docs (apache#13561)
- Clarifies version args for installing 1.10 in Docker (apache#12875)
linuxfft added a commit to linuxfft/airflow that referenced this pull request Aug 13, 2021
Apache Airflow 1.10.15

- Fix `airflow db upgrade` to upgrade db as intended (apache#13267)
- Moved boto3 limitation to snowflake (apache#13286)
- `KubernetesExecutor` should accept images from `executor_config` (apache#13074)
- Scheduler should acknowledge active runs properly (apache#13803)
- Bugfix: Unable to import Airflow plugins on Python 3.8 (apache#12859)
- Include `airflow/contrib/executors` in the dist package
- Pin Click version for Python 2.7 users
- Ensure all statsd timers use millisecond values. (apache#10633)
- [`kubernetes_generate_dag_yaml`] - Fix dag yaml generate function (apache#13816)
- Fix `airflow tasks clear` cli command wirh `--yes` (apache#14188)
- Fix permission error on non-POSIX filesystem (apache#13121) (apache#14383)
- Fixed deprecation message for "variables" command (apache#14457)
- BugFix: fix the `delete_dag` function of json_client (apache#14441)
- Fix merging of secrets and configmaps for `KubernetesExecutor` (apache#14090)
- Fix webserver exiting when gunicorn master crashes (apache#13470)
- Bump ini from 1.3.5 to 1.3.8 in `airflow/www_rbac`
- Bump datatables.net from 1.10.21 to 1.10.23 in `airflow/www_rbac`
- Webserver: Sanitize string passed to origin param (apache#14738)
- Make `rbac_app`'s `db.session` use the same timezone with `@provide_session` (apache#14025)

- Adds airflow as viable docker command in official image (apache#12878)
- `StreamLogWriter`: Provide (no-op) close method (apache#10885)
- Add 'airflow variables list' command for 1.10.x transition version (apache#14462)

- Update URL for Airflow docs (apache#13561)
- Clarifies version args for installing 1.10 in Docker (apache#12875)
andrewdanks added a commit to Affirm/airflow that referenced this pull request Mar 18, 2022
Apache Airflow 1.10.15

- Fix `airflow db upgrade` to upgrade db as intended (apache#13267)
- Moved boto3 limitation to snowflake (apache#13286)
- `KubernetesExecutor` should accept images from `executor_config` (apache#13074)
- Scheduler should acknowledge active runs properly (apache#13803)
- Bugfix: Unable to import Airflow plugins on Python 3.8 (apache#12859)
- Include `airflow/contrib/executors` in the dist package
- Pin Click version for Python 2.7 users
- Ensure all statsd timers use millisecond values. (apache#10633)
- [`kubernetes_generate_dag_yaml`] - Fix dag yaml generate function (apache#13816)
- Fix `airflow tasks clear` cli command wirh `--yes` (apache#14188)
- Fix permission error on non-POSIX filesystem (apache#13121) (apache#14383)
- Fixed deprecation message for "variables" command (apache#14457)
- BugFix: fix the `delete_dag` function of json_client (apache#14441)
- Fix merging of secrets and configmaps for `KubernetesExecutor` (apache#14090)
- Fix webserver exiting when gunicorn master crashes (apache#13470)
- Bump ini from 1.3.5 to 1.3.8 in `airflow/www_rbac`
- Bump datatables.net from 1.10.21 to 1.10.23 in `airflow/www_rbac`
- Webserver: Sanitize string passed to origin param (apache#14738)
- Make `rbac_app`'s `db.session` use the same timezone with `@provide_session` (apache#14025)

- Adds airflow as viable docker command in official image (apache#12878)
- `StreamLogWriter`: Provide (no-op) close method (apache#10885)
- Add 'airflow variables list' command for 1.10.x transition version (apache#14462)

- Update URL for Airflow docs (apache#13561)
- Clarifies version args for installing 1.10 in Docker (apache#12875)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:CLI full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants