Re: [PR] Add option --version-suffix-for-local [airflow]
perry2of5 commented on PR #43769: URL: https://github.com/apache/airflow/pull/43769#issuecomment-2483489023 Thanks for the help, @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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
potiuk merged PR #43769: URL: https://github.com/apache/airflow/pull/43769 -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
perry2of5 commented on code in PR #43769: URL: https://github.com/apache/airflow/pull/43769#discussion_r1845211393 ## contributing-docs/11_provider_packages.rst: ## @@ -122,19 +122,21 @@ Local Development Release of a Specific Provider When you develop a provider, you can release it locally and test it in your Airflow environment. This should -be accomplished using breeze. Choose a suffix for the release such as "dev1" and run the breeze build for +be accomplished using breeze. Choose a suffix for the release such as "+patch.asb.1" and run the breeze build for that provider. Remember Provider IDs use a dot ('.') for directory separators so the Provider ID for the Microsoft Azure provider is 'microsoft.azure'. This can be provided in the PACKAGE_LIST environment variable or passed on the command line. -``export PACKAGE_LIST=microsoft.azure`` +```bash +export PACKAGE_LIST=microsoft.azure + Then build the provider (you don't need to pass the package ID if you set the environment variable above): ```bash breeze release-management prepare-provider-packages \ ---package-format both --version-suffix-for-pypi=dev1 \ ---skip-tag-check microsoft.azure +--package-format both --version-suffix-for-local=+patch.asb.1 \ Review Comment: I believe I added the old example you wanted back in. -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
potiuk commented on code in PR #43769: URL: https://github.com/apache/airflow/pull/43769#discussion_r1844898371 ## contributing-docs/11_provider_packages.rst: ## @@ -122,19 +122,21 @@ Local Development Release of a Specific Provider When you develop a provider, you can release it locally and test it in your Airflow environment. This should -be accomplished using breeze. Choose a suffix for the release such as "dev1" and run the breeze build for +be accomplished using breeze. Choose a suffix for the release such as "+patch.asb.1" and run the breeze build for that provider. Remember Provider IDs use a dot ('.') for directory separators so the Provider ID for the Microsoft Azure provider is 'microsoft.azure'. This can be provided in the PACKAGE_LIST environment variable or passed on the command line. -``export PACKAGE_LIST=microsoft.azure`` +```bash +export PACKAGE_LIST=microsoft.azure + Then build the provider (you don't need to pass the package ID if you set the environment variable above): ```bash breeze release-management prepare-provider-packages \ ---package-format both --version-suffix-for-pypi=dev1 \ ---skip-tag-check microsoft.azure +--package-format both --version-suffix-for-local=+patch.asb.1 \ Review Comment: Looks goood. Indeed - just doc leaving old example would be great. -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
potiuk commented on code in PR #43769: URL: https://github.com/apache/airflow/pull/43769#discussion_r1844898320 ## dev/breeze/src/airflow_breeze/utils/packages.py: ## @@ -420,7 +420,12 @@ def get_dist_package_name_prefix(provider_id: str) -> str: def apply_version_suffix(install_clause: str, version_suffix: str) -> str: -if install_clause.startswith("apache-airflow") and ">=" in install_clause and version_suffix: +if ( +install_clause.startswith("apache-airflow") +and ">=" in install_clause +and version_suffix +and not is_local_version_suffix(version_suffix) Review Comment: Looks goood. Indeed - just doc leaving old example would be great. -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
perry2of5 commented on code in PR #43769: URL: https://github.com/apache/airflow/pull/43769#discussion_r1843060626 ## dev/breeze/src/airflow_breeze/commands/release_management_commands.py: ## @@ -926,7 +936,8 @@ def prepare_provider_packages( include_removed=include_removed_providers, include_not_ready=include_not_ready_providers, ) -if not skip_tag_check: +package_version = create_package_version(version_suffix_for_pypi, version_suffix_for_local) +if not skip_tag_check and not is_local_package_version(package_version): run_command(["git", "remote", "rm", "apache-https-for-providers"], check=False, stderr=DEVNULL) make_sure_remote_apache_exists_and_fetch(github_repository=github_repository) Review Comment: Thank you. That was what my initial analysis showed, but I'm new to 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
potiuk commented on code in PR #43769: URL: https://github.com/apache/airflow/pull/43769#discussion_r1843049932 ## dev/breeze/src/airflow_breeze/commands/release_management_commands.py: ## @@ -926,7 +936,8 @@ def prepare_provider_packages( include_removed=include_removed_providers, include_not_ready=include_not_ready_providers, ) -if not skip_tag_check: +package_version = create_package_version(version_suffix_for_pypi, version_suffix_for_local) +if not skip_tag_check and not is_local_package_version(package_version): run_command(["git", "remote", "rm", "apache-https-for-providers"], check=False, stderr=DEVNULL) make_sure_remote_apache_exists_and_fetch(github_repository=github_repository) Review Comment: Yes. We can ignore the "git remote" command if local package.. This is only useful to skip providers automatically when they are already released. -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
potiuk commented on code in PR #43769: URL: https://github.com/apache/airflow/pull/43769#discussion_r1843049932 ## dev/breeze/src/airflow_breeze/commands/release_management_commands.py: ## @@ -926,7 +936,8 @@ def prepare_provider_packages( include_removed=include_removed_providers, include_not_ready=include_not_ready_providers, ) -if not skip_tag_check: +package_version = create_package_version(version_suffix_for_pypi, version_suffix_for_local) +if not skip_tag_check and not is_local_package_version(package_version): run_command(["git", "remote", "rm", "apache-https-for-providers"], check=False, stderr=DEVNULL) make_sure_remote_apache_exists_and_fetch(github_repository=github_repository) Review Comment: Yes. We can skip it. This is only useful to skip providers automatically when they are already released. -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
perry2of5 commented on code in PR #43769: URL: https://github.com/apache/airflow/pull/43769#discussion_r1843040356 ## dev/breeze/src/airflow_breeze/commands/release_management_commands.py: ## @@ -926,7 +936,8 @@ def prepare_provider_packages( include_removed=include_removed_providers, include_not_ready=include_not_ready_providers, ) -if not skip_tag_check: +package_version = create_package_version(version_suffix_for_pypi, version_suffix_for_local) +if not skip_tag_check and not is_local_package_version(package_version): run_command(["git", "remote", "rm", "apache-https-for-providers"], check=False, stderr=DEVNULL) make_sure_remote_apache_exists_and_fetch(github_repository=github_repository) Review Comment: I'm not totally convinced we want to ignore the github_repository flag if it is a local build any more. When it was just a local provider build, it seemed okay. Now I have concerns and I'm not sure if I need to investigate this. Any thoughts, @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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
perry2of5 commented on code in PR #43769: URL: https://github.com/apache/airflow/pull/43769#discussion_r1843039274 ## contributing-docs/11_provider_packages.rst: ## @@ -122,19 +122,21 @@ Local Development Release of a Specific Provider When you develop a provider, you can release it locally and test it in your Airflow environment. This should -be accomplished using breeze. Choose a suffix for the release such as "dev1" and run the breeze build for +be accomplished using breeze. Choose a suffix for the release such as "+patch.asb.1" and run the breeze build for that provider. Remember Provider IDs use a dot ('.') for directory separators so the Provider ID for the Microsoft Azure provider is 'microsoft.azure'. This can be provided in the PACKAGE_LIST environment variable or passed on the command line. -``export PACKAGE_LIST=microsoft.azure`` +```bash +export PACKAGE_LIST=microsoft.azure + Then build the provider (you don't need to pass the package ID if you set the environment variable above): ```bash breeze release-management prepare-provider-packages \ ---package-format both --version-suffix-for-pypi=dev1 \ ---skip-tag-check microsoft.azure +--package-format both --version-suffix-for-local=+patch.asb.1 \ Review Comment: I believe I addressed this in the code, but not in the docs yet. -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
perry2of5 commented on code in PR #43769: URL: https://github.com/apache/airflow/pull/43769#discussion_r1843039113 ## dev/breeze/src/airflow_breeze/utils/packages.py: ## @@ -420,7 +420,12 @@ def get_dist_package_name_prefix(provider_id: str) -> str: def apply_version_suffix(install_clause: str, version_suffix: str) -> str: -if install_clause.startswith("apache-airflow") and ">=" in install_clause and version_suffix: +if ( +install_clause.startswith("apache-airflow") +and ">=" in install_clause +and version_suffix +and not is_local_version_suffix(version_suffix) Review Comment: I changed this to ignore the local suffix, if any, and process this if there is a PyPi suffix. -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
perry2of5 commented on PR #43769: URL: https://github.com/apache/airflow/pull/43769#issuecomment-2469489823 I like this suggestion. I hadn’t thought of people verifying the release, but I see the value. Thanks for pointing this out. I will implement this suggestion. Probably Wednesday or Thursday PST. -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
potiuk commented on code in PR #43769: URL: https://github.com/apache/airflow/pull/43769#discussion_r1836728137 ## dev/breeze/src/airflow_breeze/utils/packages.py: ## @@ -420,7 +420,12 @@ def get_dist_package_name_prefix(provider_id: str) -> str: def apply_version_suffix(install_clause: str, version_suffix: str) -> str: -if install_clause.startswith("apache-airflow") and ">=" in install_clause and version_suffix: +if ( +install_clause.startswith("apache-airflow") +and ">=" in install_clause +and version_suffix +and not is_local_version_suffix(version_suffix) Review Comment: Hmm. I think we should support BOTH pypi and local suffix. It's really likely that we want to have rc1+local-tag -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add option --version-suffix-for-local [airflow]
potiuk commented on code in PR #43769: URL: https://github.com/apache/airflow/pull/43769#discussion_r1836726117 ## contributing-docs/11_provider_packages.rst: ## @@ -122,19 +122,21 @@ Local Development Release of a Specific Provider When you develop a provider, you can release it locally and test it in your Airflow environment. This should -be accomplished using breeze. Choose a suffix for the release such as "dev1" and run the breeze build for +be accomplished using breeze. Choose a suffix for the release such as "+patch.asb.1" and run the breeze build for that provider. Remember Provider IDs use a dot ('.') for directory separators so the Provider ID for the Microsoft Azure provider is 'microsoft.azure'. This can be provided in the PACKAGE_LIST environment variable or passed on the command line. -``export PACKAGE_LIST=microsoft.azure`` +```bash +export PACKAGE_LIST=microsoft.azure + Then build the provider (you don't need to pass the package ID if you set the environment variable above): ```bash breeze release-management prepare-provider-packages \ ---package-format both --version-suffix-for-pypi=dev1 \ ---skip-tag-check microsoft.azure +--package-format both --version-suffix-for-local=+patch.asb.1 \ Review Comment: It would be good to separate those two cases. I see a need to generate "dev1" or "rc" locally as well as "+patchwhatever" - for example when we are verifying packages for releases, PMC members prepare the same packages locally as they will be released - in order to test "binary reproducibiility" - i.e. they verify if the package they produce locally is binary the same as the one that release manger produced (this is security feature of our release process). So it woudl be great if we show both cases AND explain a bit the difference betweeen the two - as it is not entirely obvious for someone who does not know the versioning PEP. -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org