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/12/21 22:20:09 UTC
[GitHub] [airflow] debodirno opened a new pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
debodirno opened a new pull request #13234:
URL: https://github.com/apache/airflow/pull/13234
Made changes to prepare_provider_packages.py to use argparse.
To be reviewed by : @mik-laj , @potiuk
closes: #13069
----------------------------------------------------------------
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] mik-laj commented on a change in pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#discussion_r549329586
##########
File path: dev/provider_packages/prepare_provider_packages.py
##########
@@ -1532,70 +1506,92 @@ def copy_readme_and_changelog(provider_package_id: str, backport_packages: bool)
suffix = ""
provider_names = get_provider_packages()
- possible_first_params = provider_names.copy()
- possible_first_params.append(LIST_PROVIDERS_PACKAGES)
- possible_first_params.append(LIST_BACKPORTABLE_PACKAGES)
- possible_first_params.append(UPDATE_PACKAGE_RELEASE_NOTES)
- possible_first_params.append(GENERATE_SETUP_FILES)
- if len(sys.argv) == 1:
- print(
- """
-ERROR! Missing first param"
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- if sys.argv[1] == "--version-suffix":
- if len(sys.argv) < 3:
- print(
+ help_text = textwrap.dedent(
+ """
+ Available packages:
+
"""
-ERROR! --version-suffix needs parameter!
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- suffix = sys.argv[2]
- sys.argv = [sys.argv[0]] + sys.argv[3:]
- elif "--help" in sys.argv or "-h" in sys.argv or len(sys.argv) < 2:
- usage()
- sys.exit(0)
+ )
+ out = ""
+ for package in provider_names:
+ out += f"{package} "
Review comment:
```suggestion
out = " ".join(provider_names)
```
----------------------------------------------------------------
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 #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-750828169
[The Workflow run](https://github.com/apache/airflow/actions/runs/442078032) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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] debodirno commented on pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
debodirno commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-751687085
@potiuk Looks like I am done. I have tested the breeze commands and it seems good. You can review this.
----------------------------------------------------------------
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] debodirno commented on a change in pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
debodirno commented on a change in pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#discussion_r549426097
##########
File path: dev/provider_packages/prepare_provider_packages.py
##########
@@ -1532,70 +1506,92 @@ def copy_readme_and_changelog(provider_package_id: str, backport_packages: bool)
suffix = ""
provider_names = get_provider_packages()
- possible_first_params = provider_names.copy()
- possible_first_params.append(LIST_PROVIDERS_PACKAGES)
- possible_first_params.append(LIST_BACKPORTABLE_PACKAGES)
- possible_first_params.append(UPDATE_PACKAGE_RELEASE_NOTES)
- possible_first_params.append(GENERATE_SETUP_FILES)
- if len(sys.argv) == 1:
- print(
- """
-ERROR! Missing first param"
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- if sys.argv[1] == "--version-suffix":
- if len(sys.argv) < 3:
- print(
+ help_text = textwrap.dedent(
+ """
+ Available packages:
+
"""
-ERROR! --version-suffix needs parameter!
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- suffix = sys.argv[2]
- sys.argv = [sys.argv[0]] + sys.argv[3:]
- elif "--help" in sys.argv or "-h" in sys.argv or len(sys.argv) < 2:
- usage()
- sys.exit(0)
+ )
+ out = ""
+ for package in provider_names:
+ out += f"{package} "
+ out_array = textwrap.wrap(out, 80)
- if sys.argv[1] not in possible_first_params:
- print(
- f"""
-ERROR! Wrong first param: {sys.argv[1]}
-""",
- file=sys.stderr,
- )
- usage()
- print()
+ for text in out_array:
+ help_text += f"{text}\n"
+
+ parser = argparse.ArgumentParser(description=help_text, formatter_class=argparse.RawTextHelpFormatter)
Review comment:
I can do that.
----------------------------------------------------------------
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] debodirno commented on pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
debodirno commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-754015194
> Could you please rebase @debodirno ? There was a (fixed already) timeout problem on CURL usage which manifested itself in cancelled CI checks, however if you rebase to latest master it should work fine now.
Sure thing @potiuk.
----------------------------------------------------------------
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] potiuk commented on pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-750898510
One comment before I review @debodirno . You will also have to review and double check the instructions in https://github.com/apache/airflow/blob/master/dev/README_RELEASE_PROVIDER_PACKAGES.md and https://github.com/apache/airflow/blob/master/dev/PROVIDER_PACKAGE_DETAILS.md to reflect the changes and make sure that `breeze` commands described there are working as expected.
----------------------------------------------------------------
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] debodirno commented on a change in pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
debodirno commented on a change in pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#discussion_r549396680
##########
File path: dev/provider_packages/prepare_provider_packages.py
##########
@@ -1532,70 +1506,92 @@ def copy_readme_and_changelog(provider_package_id: str, backport_packages: bool)
suffix = ""
provider_names = get_provider_packages()
- possible_first_params = provider_names.copy()
- possible_first_params.append(LIST_PROVIDERS_PACKAGES)
- possible_first_params.append(LIST_BACKPORTABLE_PACKAGES)
- possible_first_params.append(UPDATE_PACKAGE_RELEASE_NOTES)
- possible_first_params.append(GENERATE_SETUP_FILES)
- if len(sys.argv) == 1:
- print(
- """
-ERROR! Missing first param"
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- if sys.argv[1] == "--version-suffix":
- if len(sys.argv) < 3:
- print(
+ help_text = textwrap.dedent(
+ """
+ Available packages:
+
"""
-ERROR! --version-suffix needs parameter!
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- suffix = sys.argv[2]
- sys.argv = [sys.argv[0]] + sys.argv[3:]
- elif "--help" in sys.argv or "-h" in sys.argv or len(sys.argv) < 2:
- usage()
- sys.exit(0)
+ )
+ out = ""
+ for package in provider_names:
+ out += f"{package} "
Review comment:
Done.
----------------------------------------------------------------
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 #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-750877703
[The Workflow run](https://github.com/apache/airflow/actions/runs/442372091) 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] mik-laj commented on a change in pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#discussion_r549331159
##########
File path: dev/provider_packages/prepare_provider_packages.py
##########
@@ -1532,70 +1506,92 @@ def copy_readme_and_changelog(provider_package_id: str, backport_packages: bool)
suffix = ""
provider_names = get_provider_packages()
- possible_first_params = provider_names.copy()
- possible_first_params.append(LIST_PROVIDERS_PACKAGES)
- possible_first_params.append(LIST_BACKPORTABLE_PACKAGES)
- possible_first_params.append(UPDATE_PACKAGE_RELEASE_NOTES)
- possible_first_params.append(GENERATE_SETUP_FILES)
- if len(sys.argv) == 1:
- print(
- """
-ERROR! Missing first param"
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- if sys.argv[1] == "--version-suffix":
- if len(sys.argv) < 3:
- print(
+ help_text = textwrap.dedent(
+ """
+ Available packages:
+
"""
-ERROR! --version-suffix needs parameter!
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- suffix = sys.argv[2]
- sys.argv = [sys.argv[0]] + sys.argv[3:]
- elif "--help" in sys.argv or "-h" in sys.argv or len(sys.argv) < 2:
- usage()
- sys.exit(0)
+ )
+ out = ""
+ for package in provider_names:
+ out += f"{package} "
+ out_array = textwrap.wrap(out, 80)
- if sys.argv[1] not in possible_first_params:
- print(
- f"""
-ERROR! Wrong first param: {sys.argv[1]}
-""",
- file=sys.stderr,
- )
- usage()
- print()
+ for text in out_array:
+ help_text += f"{text}\n"
+
+ parser = argparse.ArgumentParser(description=help_text, formatter_class=argparse.RawTextHelpFormatter)
+ parser.add_argument(
+ PACKAGES,
+ help=textwrap.dedent(
+ """provide packages for setup.py.
+Choose from the above available packages."""
+ ),
+ )
+ parser.add_argument(
+ VERSION_SUFFIX,
+ metavar="SUFFIX",
+ help=textwrap.dedent(
+ """adds version suffix to version of the packages.
+Only useful when generating RC candidates for PyPI."""
+ ),
+ )
+
+ subparsers = parser.add_subparsers(dest="cmd")
+
+ first_param_subparser1 = subparsers.add_parser(LIST_PROVIDERS_PACKAGES, help="list all provider packages")
+ first_param_subparser1.set_defaults(cmd=LIST_PROVIDERS_PACKAGES)
Review comment:
Do we need these constants? In the [original issue](https://github.com/apache/airflow/issues/13069), there is a code attached that proposes to use the function instead. This looks a little more readable as the docstring can then be used as a command description
https://gist.github.com/mik-laj/ff008718fc6cec9fe929731b8c62d6f8
----------------------------------------------------------------
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 #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-753532260
[The Workflow run](https://github.com/apache/airflow/actions/runs/458219496) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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] potiuk commented on a change in pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#discussion_r549448941
##########
File path: dev/provider_packages/prepare_provider_packages.py
##########
@@ -1532,70 +1506,92 @@ def copy_readme_and_changelog(provider_package_id: str, backport_packages: bool)
suffix = ""
provider_names = get_provider_packages()
- possible_first_params = provider_names.copy()
- possible_first_params.append(LIST_PROVIDERS_PACKAGES)
- possible_first_params.append(LIST_BACKPORTABLE_PACKAGES)
- possible_first_params.append(UPDATE_PACKAGE_RELEASE_NOTES)
- possible_first_params.append(GENERATE_SETUP_FILES)
- if len(sys.argv) == 1:
- print(
- """
-ERROR! Missing first param"
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- if sys.argv[1] == "--version-suffix":
- if len(sys.argv) < 3:
- print(
+ help_text = textwrap.dedent(
+ """
+ Available packages:
+
"""
-ERROR! --version-suffix needs parameter!
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- suffix = sys.argv[2]
- sys.argv = [sys.argv[0]] + sys.argv[3:]
- elif "--help" in sys.argv or "-h" in sys.argv or len(sys.argv) < 2:
- usage()
- sys.exit(0)
+ )
+ out = ""
+ for package in provider_names:
+ out += f"{package} "
+ out_array = textwrap.wrap(out, 80)
- if sys.argv[1] not in possible_first_params:
- print(
- f"""
-ERROR! Wrong first param: {sys.argv[1]}
-""",
- file=sys.stderr,
- )
- usage()
- print()
+ for text in out_array:
+ help_text += f"{text}\n"
+
+ parser = argparse.ArgumentParser(description=help_text, formatter_class=argparse.RawTextHelpFormatter)
+ parser.add_argument(
+ PACKAGES,
+ help=textwrap.dedent(
+ """provide packages for setup.py.
+Choose from the above available packages."""
+ ),
+ )
+ parser.add_argument(
+ VERSION_SUFFIX,
+ metavar="SUFFIX",
+ help=textwrap.dedent(
+ """adds version suffix to version of the packages.
+Only useful when generating RC candidates for PyPI."""
+ ),
+ )
+
+ subparsers = parser.add_subparsers(dest="cmd")
+
+ first_param_subparser1 = subparsers.add_parser(LIST_PROVIDERS_PACKAGES, help="list all provider packages")
+ first_param_subparser1.set_defaults(cmd=LIST_PROVIDERS_PACKAGES)
Review comment:
Yep. I agree with @mik-laj this is a much better approach. It makes it much more readable if the code to execute is in separate functions for each options. It's pretty natural evolution when the command line options become more complex. When you start, it's easier to add hand-written stuff, and keep code "together" but once you start adding functionality there is a need at some point in time (now !) to split it into smaller, independent methods. The next step is to split it into separate modules, but we are far from that!
----------------------------------------------------------------
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] debodirno commented on pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
debodirno commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-750903527
> One comment before I review @debodirno . You will also have to review and double check the instructions in https://github.com/apache/airflow/blob/master/dev/README_RELEASE_PROVIDER_PACKAGES.md and https://github.com/apache/airflow/blob/master/dev/PROVIDER_PACKAGE_DETAILS.md to reflect the changes and make sure that `breeze` commands described there are working as expected.
Okay @potiuk. I have not yet tested it thoroughly. Just that, I have grepped through the codebase to find the usages and made changes. But yes, I will test this out based on the instructions in these markdown files.
----------------------------------------------------------------
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] potiuk commented on pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-754000414
Could you please rebase @debodirno ? There was a (fixed already) timeout problem on CURL usage which manifested itself in cancelled CI checks, however if you rebase to latest master it should work fine now.
----------------------------------------------------------------
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] mik-laj commented on a change in pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#discussion_r549496513
##########
File path: dev/provider_packages/prepare_provider_packages.py
##########
@@ -1532,70 +1506,92 @@ def copy_readme_and_changelog(provider_package_id: str, backport_packages: bool)
suffix = ""
provider_names = get_provider_packages()
- possible_first_params = provider_names.copy()
- possible_first_params.append(LIST_PROVIDERS_PACKAGES)
- possible_first_params.append(LIST_BACKPORTABLE_PACKAGES)
- possible_first_params.append(UPDATE_PACKAGE_RELEASE_NOTES)
- possible_first_params.append(GENERATE_SETUP_FILES)
- if len(sys.argv) == 1:
- print(
- """
-ERROR! Missing first param"
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- if sys.argv[1] == "--version-suffix":
- if len(sys.argv) < 3:
- print(
+ help_text = textwrap.dedent(
+ """
+ Available packages:
+
"""
-ERROR! --version-suffix needs parameter!
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- suffix = sys.argv[2]
- sys.argv = [sys.argv[0]] + sys.argv[3:]
- elif "--help" in sys.argv or "-h" in sys.argv or len(sys.argv) < 2:
- usage()
- sys.exit(0)
+ )
+ out = ""
+ for package in provider_names:
+ out += f"{package} "
+ out_array = textwrap.wrap(out, 80)
- if sys.argv[1] not in possible_first_params:
- print(
- f"""
-ERROR! Wrong first param: {sys.argv[1]}
-""",
- file=sys.stderr,
- )
- usage()
- print()
+ for text in out_array:
+ help_text += f"{text}\n"
+
+ parser = argparse.ArgumentParser(description=help_text, formatter_class=argparse.RawTextHelpFormatter)
+ parser.add_argument(
+ PACKAGES,
+ help=textwrap.dedent(
+ """provide packages for setup.py.
+Choose from the above available packages."""
+ ),
+ )
+ parser.add_argument(
+ VERSION_SUFFIX,
+ metavar="SUFFIX",
+ help=textwrap.dedent(
+ """adds version suffix to version of the packages.
+Only useful when generating RC candidates for PyPI."""
+ ),
+ )
+
+ subparsers = parser.add_subparsers(dest="cmd")
+
+ first_param_subparser1 = subparsers.add_parser(LIST_PROVIDERS_PACKAGES, help="list all provider packages")
+ first_param_subparser1.set_defaults(cmd=LIST_PROVIDERS_PACKAGES)
Review comment:
> So do you want me to create functions for each of the arguments
I think it would be a good idea to create a new function for **each subcommand** (not argument).
----------------------------------------------------------------
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] potiuk commented on pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-754200130
Cool !
----------------------------------------------------------------
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] potiuk merged pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #13234:
URL: https://github.com/apache/airflow/pull/13234
----------------------------------------------------------------
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] debodirno commented on pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
debodirno commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-753521565
Hey @mik-laj and @potiuk sorry, got delayed because of no internet connectivity as I was travelling. Updating the PR today itself.
----------------------------------------------------------------
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] debodirno commented on a change in pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
debodirno commented on a change in pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#discussion_r549396775
##########
File path: dev/provider_packages/prepare_provider_packages.py
##########
@@ -1532,70 +1506,92 @@ def copy_readme_and_changelog(provider_package_id: str, backport_packages: bool)
suffix = ""
provider_names = get_provider_packages()
- possible_first_params = provider_names.copy()
- possible_first_params.append(LIST_PROVIDERS_PACKAGES)
- possible_first_params.append(LIST_BACKPORTABLE_PACKAGES)
- possible_first_params.append(UPDATE_PACKAGE_RELEASE_NOTES)
- possible_first_params.append(GENERATE_SETUP_FILES)
- if len(sys.argv) == 1:
- print(
- """
-ERROR! Missing first param"
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- if sys.argv[1] == "--version-suffix":
- if len(sys.argv) < 3:
- print(
+ help_text = textwrap.dedent(
Review comment:
Done.
----------------------------------------------------------------
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] mik-laj commented on a change in pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#discussion_r549331656
##########
File path: dev/provider_packages/prepare_provider_packages.py
##########
@@ -1532,70 +1506,92 @@ def copy_readme_and_changelog(provider_package_id: str, backport_packages: bool)
suffix = ""
provider_names = get_provider_packages()
- possible_first_params = provider_names.copy()
- possible_first_params.append(LIST_PROVIDERS_PACKAGES)
- possible_first_params.append(LIST_BACKPORTABLE_PACKAGES)
- possible_first_params.append(UPDATE_PACKAGE_RELEASE_NOTES)
- possible_first_params.append(GENERATE_SETUP_FILES)
- if len(sys.argv) == 1:
- print(
- """
-ERROR! Missing first param"
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- if sys.argv[1] == "--version-suffix":
- if len(sys.argv) < 3:
- print(
+ help_text = textwrap.dedent(
+ """
+ Available packages:
+
"""
-ERROR! --version-suffix needs parameter!
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- suffix = sys.argv[2]
- sys.argv = [sys.argv[0]] + sys.argv[3:]
- elif "--help" in sys.argv or "-h" in sys.argv or len(sys.argv) < 2:
- usage()
- sys.exit(0)
+ )
+ out = ""
+ for package in provider_names:
+ out += f"{package} "
+ out_array = textwrap.wrap(out, 80)
- if sys.argv[1] not in possible_first_params:
- print(
- f"""
-ERROR! Wrong first param: {sys.argv[1]}
-""",
- file=sys.stderr,
- )
- usage()
- print()
+ for text in out_array:
+ help_text += f"{text}\n"
+
+ parser = argparse.ArgumentParser(description=help_text, formatter_class=argparse.RawTextHelpFormatter)
Review comment:
A common practice is to create a get_parser / _get_parser function that returns this object. This way, the functions are broken down into several parts and the code is a bit more readable.
----------------------------------------------------------------
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] mik-laj commented on a change in pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#discussion_r549330084
##########
File path: dev/provider_packages/prepare_provider_packages.py
##########
@@ -1532,70 +1506,92 @@ def copy_readme_and_changelog(provider_package_id: str, backport_packages: bool)
suffix = ""
provider_names = get_provider_packages()
- possible_first_params = provider_names.copy()
- possible_first_params.append(LIST_PROVIDERS_PACKAGES)
- possible_first_params.append(LIST_BACKPORTABLE_PACKAGES)
- possible_first_params.append(UPDATE_PACKAGE_RELEASE_NOTES)
- possible_first_params.append(GENERATE_SETUP_FILES)
- if len(sys.argv) == 1:
- print(
- """
-ERROR! Missing first param"
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- if sys.argv[1] == "--version-suffix":
- if len(sys.argv) < 3:
- print(
+ help_text = textwrap.dedent(
Review comment:
Are you sure we need textwrap.dedent here? It looks like it shouldn't be a problem to put it on one line.
----------------------------------------------------------------
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] mik-laj commented on pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-753481292
Do you need help? I would be happy if we could complete this PR as it is needed for https://github.com/apache/airflow/issues/13230 and https://github.com/apache/airflow/issues/13273
----------------------------------------------------------------
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 #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-753731837
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] github-actions[bot] commented on pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-750894380
[The Workflow run](https://github.com/apache/airflow/actions/runs/442104453) 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] debodirno commented on a change in pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
debodirno commented on a change in pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#discussion_r549426011
##########
File path: dev/provider_packages/prepare_provider_packages.py
##########
@@ -1532,70 +1506,92 @@ def copy_readme_and_changelog(provider_package_id: str, backport_packages: bool)
suffix = ""
provider_names = get_provider_packages()
- possible_first_params = provider_names.copy()
- possible_first_params.append(LIST_PROVIDERS_PACKAGES)
- possible_first_params.append(LIST_BACKPORTABLE_PACKAGES)
- possible_first_params.append(UPDATE_PACKAGE_RELEASE_NOTES)
- possible_first_params.append(GENERATE_SETUP_FILES)
- if len(sys.argv) == 1:
- print(
- """
-ERROR! Missing first param"
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- if sys.argv[1] == "--version-suffix":
- if len(sys.argv) < 3:
- print(
+ help_text = textwrap.dedent(
+ """
+ Available packages:
+
"""
-ERROR! --version-suffix needs parameter!
-""",
- file=sys.stderr,
- )
- usage()
- sys.exit(1)
- suffix = sys.argv[2]
- sys.argv = [sys.argv[0]] + sys.argv[3:]
- elif "--help" in sys.argv or "-h" in sys.argv or len(sys.argv) < 2:
- usage()
- sys.exit(0)
+ )
+ out = ""
+ for package in provider_names:
+ out += f"{package} "
+ out_array = textwrap.wrap(out, 80)
- if sys.argv[1] not in possible_first_params:
- print(
- f"""
-ERROR! Wrong first param: {sys.argv[1]}
-""",
- file=sys.stderr,
- )
- usage()
- print()
+ for text in out_array:
+ help_text += f"{text}\n"
+
+ parser = argparse.ArgumentParser(description=help_text, formatter_class=argparse.RawTextHelpFormatter)
+ parser.add_argument(
+ PACKAGES,
+ help=textwrap.dedent(
+ """provide packages for setup.py.
+Choose from the above available packages."""
+ ),
+ )
+ parser.add_argument(
+ VERSION_SUFFIX,
+ metavar="SUFFIX",
+ help=textwrap.dedent(
+ """adds version suffix to version of the packages.
+Only useful when generating RC candidates for PyPI."""
+ ),
+ )
+
+ subparsers = parser.add_subparsers(dest="cmd")
+
+ first_param_subparser1 = subparsers.add_parser(LIST_PROVIDERS_PACKAGES, help="list all provider packages")
+ first_param_subparser1.set_defaults(cmd=LIST_PROVIDERS_PACKAGES)
Review comment:
So do you want me to create functions for each of the arguments and put the help text as docstrings for the respective functions?
----------------------------------------------------------------
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] debodirno commented on pull request #13234: Rewrite handwritten argument parser in prepare_provider_packages.py
Posted by GitBox <gi...@apache.org>.
debodirno commented on pull request #13234:
URL: https://github.com/apache/airflow/pull/13234#issuecomment-754147990
@potiuk all the tests have run successfully. Can you merge this?
----------------------------------------------------------------
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