Re: [PR] Replace to broad exceptions into the Dev [airflow]

Taragolis merged PR #38489: URL: https://github.com/apache/airflow/pull/38489 -- 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...@airflo

Re: [PR] Replace to broad exceptions into the Dev [airflow]

Taragolis commented on PR #38489: URL: https://github.com/apache/airflow/pull/38489#issuecomment-2034653878 The error in the tests not related to this changes -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL abo

Re: [PR] Replace to broad exceptions into the Dev [airflow]

Taragolis commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1549360725 ## docs/exts/docs_build/docs_builder.py: ## @@ -88,21 +87,6 @@ def log_build_warning_filename(self) -> str: """Warnings from build job.""" return

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on PR #38489: URL: https://github.com/apache/airflow/pull/38489#issuecomment-2032734953 Should we just complete thaat one :) ? -- 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 th

Re: [PR] Replace to broad exceptions into the Dev [airflow]

Taragolis commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1542066894 ## docs/exts/provider_yaml_utils.py: ## @@ -68,7 +68,7 @@ def load_package_data(include_suspended: bool = False) -> list[dict[str, Any]]: try:

Re: [PR] Replace to broad exceptions into the Dev [airflow]

Taragolis commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1542062637 ## scripts/ci/pre_commit/pre_commit_json_schema.py: ## @@ -30,7 +30,7 @@ from jsonschema.validators import extend, validator_for if __name__ != "__main__": -

Re: [PR] Replace to broad exceptions into the Dev [airflow]

uranusjr commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1540174193 ## scripts/ci/pre_commit/pre_commit_json_schema.py: ## @@ -30,7 +30,7 @@ from jsonschema.validators import extend, validator_for if __name__ != "__main__": -

Re: [PR] Replace to broad exceptions into the Dev [airflow]

uranusjr commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1540172298 ## scripts/ci/pre_commit/pre_commit_insert_extras.py: ## @@ -81,7 +81,7 @@ def process_documentation_files() -> bool: extra_type_dict = get_extra_types_dict()

Re: [PR] Replace to broad exceptions into the Dev [airflow]

uranusjr commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1540171555 ## scripts/ci/pre_commit/pre_commit_insert_extras.py: ## @@ -42,7 +42,7 @@ def get_header_and_footer(extra_type: str, file_format: str) -> tuple[str, str]: eli

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539706545 ## dev/breeze/src/airflow_breeze/utils/packages.py: ## @@ -133,7 +133,7 @@ def refresh_provider_metadata_from_yaml_file(provider_yaml_path: Path): try:

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on PR #38489: URL: https://github.com/apache/airflow/pull/38489#issuecomment-2020968943 Added my suggestions. @uranusjr -> maybe you can review those and say what you think? -- This is an automated message from the Apache Git Service. To respond to the message, please lo

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539723940 ## scripts/in_container/update_quarantined_test_status.py: ## @@ -202,14 +202,14 @@ def get_table(history_map: dict[str, TestHistory]) -> str: print(f"Token: {tok

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539723544 ## scripts/in_container/update_quarantined_test_status.py: ## @@ -202,14 +202,14 @@ def get_table(history_map: dict[str, TestHistory]) -> str: print(f"Token: {tok

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539721623 ## scripts/ci/pre_commit/pre_commit_update_chart_dependencies.py: ## @@ -36,7 +36,7 @@ def get_latest_prometheus_statsd_exporter_version() -> str: for version in

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539722263 ## scripts/ci/pre_commit/pre_commit_update_versions.py: ## @@ -31,7 +31,7 @@ def update_version(pattern: re.Pattern, v: str, file_path: Path): with file_path.ope

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539720688 ## scripts/ci/pre_commit/pre_commit_insert_extras.py: ## @@ -81,7 +81,7 @@ def process_documentation_files() -> bool: extra_type_dict = get_extra_types_dict()

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539719095 ## scripts/ci/pre_commit/pre_commit_insert_extras.py: ## @@ -42,7 +42,7 @@ def get_header_and_footer(extra_type: str, file_format: str) -> tuple[str, str]: elif

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539718063 ## scripts/ci/pre_commit/pre_commit_generate_pypi_readme.py: ## @@ -54,7 +54,7 @@ def extract_section(content, section_name): if section_match: return se

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539717430 ## hatch_build.py: ## @@ -574,7 +574,7 @@ def get_provider_requirement(provider_spec: str) -> str: if PROVIDER_DEPENDENCIES[provider_id]["state"] not in ["ready",

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539715461 ## docs/exts/provider_yaml_utils.py: ## @@ -68,7 +68,7 @@ def load_package_data(include_suspended: bool = False) -> list[dict[str, Any]]: try: j

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539710120 ## docs/exts/docs_build/docs_builder.py: ## @@ -91,7 +91,7 @@ def log_build_warning_filename(self) -> str: @property def _current_version(self): if

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539710886 ## docs/exts/operators_and_hooks_ref.py: ## @@ -69,10 +69,10 @@ def _render_template(template_name, **kwargs): def _docs_path(filepath: str): if not filepath.s

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539709321 ## docs/conf.py: ## @@ -72,7 +72,7 @@ if provider_yaml["package-name"] == PACKAGE_NAME ) except StopIteration: -raise Exception(f"Co

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539707914 ## dev/breeze/src/airflow_breeze/utils/packages.py: ## @@ -177,7 +177,7 @@ def validate_provider_info_with_runtime_schema(provider_info: dict[str, Any]) -> j

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539706545 ## dev/breeze/src/airflow_breeze/utils/packages.py: ## @@ -133,7 +133,7 @@ def refresh_provider_metadata_from_yaml_file(provider_yaml_path: Path): try:

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539705829 ## dev/breeze/src/airflow_breeze/utils/packages.py: ## @@ -133,7 +133,7 @@ def refresh_provider_metadata_from_yaml_file(provider_yaml_path: Path): try:

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539703854 ## dev/breeze/src/airflow_breeze/utils/docs_publisher.py: ## @@ -58,15 +58,15 @@ def _build_dir(self) -> str: @property def _current_version(self):

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539704841 ## dev/breeze/src/airflow_breeze/utils/kubernetes_utils.py: ## @@ -92,7 +92,7 @@ def get_architecture_string_for_urls() -> str: return "amd64" if archite

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539704361 ## dev/breeze/src/airflow_breeze/utils/docs_publisher.py: ## @@ -58,15 +58,15 @@ def _build_dir(self) -> str: @property def _current_version(self):

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539702013 ## dev/breeze/src/airflow_breeze/commands/release_management_commands.py: ## @@ -1216,7 +1216,7 @@ def install_provider_packages( if dependency no

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539701523 ## dev/breeze/src/airflow_breeze/commands/release_management_commands.py: ## @@ -1066,7 +1066,7 @@ def _get_all_providers_in_dist( for file in DIST_DIR.glob(f"{fi

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on PR #38489: URL: https://github.com/apache/airflow/pull/38489#issuecomment-2020937536 BTW. If we want to be **REALLY** correct here - every time we send a ValueError (i.e. user provided a value that is wrong), we should do: ``` get_console().print("[error]The va

Re: [PR] Replace to broad exceptions into the Dev [airflow]

potiuk commented on PR #38489: URL: https://github.com/apache/airflow/pull/38489#issuecomment-2020926756 Answering for @uranusjr I think possibly yes - those that do not print value that is wrong, could be runtime errors. This is completely incosequential though in dev tools, because those

Re: [PR] Replace to broad exceptions into the Dev [airflow]

Taragolis commented on PR #38489: URL: https://github.com/apache/airflow/pull/38489#issuecomment-2020152436 > I feel some of the exceptions shouldn’t be ValueError. This type is for when a value is not right, but some of those aren’t specifically pointing to a value. RuntimeError would be b

Re: [PR] Replace to broad exceptions into the Dev [airflow]

uranusjr commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539027899 ## dev/breeze/src/airflow_breeze/global_constants.py: ## @@ -360,7 +360,7 @@ def get_airflow_version(): airflow_version = line.split()[2][1:-1]

Re: [PR] Replace to broad exceptions into the Dev [airflow]

uranusjr commented on code in PR #38489: URL: https://github.com/apache/airflow/pull/38489#discussion_r1539027899 ## dev/breeze/src/airflow_breeze/global_constants.py: ## @@ -360,7 +360,7 @@ def get_airflow_version(): airflow_version = line.split()[2][1:-1]

Re: [PR] Replace to broad exceptions into the Dev [airflow]

uranusjr commented on PR #38489: URL: https://github.com/apache/airflow/pull/38489#issuecomment-2020142225 I feel some of the exceptions shouldn’t be ValueError. This type is for when a value is not right, but some of those aren’t specifically pointing to a value. RuntimeError would be bett

[PR] Replace to broad exceptions into the Dev [airflow]

Taragolis opened a new pull request, #38489: URL: https://github.com/apache/airflow/pull/38489 Similar to: https://github.com/apache/airflow/pull/38344 This one less affect the tests but still better to use custom or other more appropriate builtin exceptions rather tha