Fix WASB remote logging base path handling (#58946)#61013
Fix WASB remote logging base path handling (#58946)#61013jason810496 merged 8 commits intoapache:mainfrom
Conversation
kalluripradeep
left a comment
There was a problem hiding this comment.
Nice fix! This addresses a real issue with the current documentation guidance.
The solution is clean and backward-compatible - stripping the wasb:// prefix while still supporting the legacy wasb- format is the right approach.
I agree with @SameerMesiah97 suggestion to add a comment explaining the normalization. Something like:
if remote_base_log_folder.startswith("wasb://"):
# Strip wasb:// scheme to get clean blob path
# Docs recommend wasb://container format but os.path.join treats scheme as part of path
wasb_remote_base = remote_base_log_folder.removeprefix("wasb://")
else:
# Support legacy wasb- prefix format for backward compatibility
wasb_remote_base = remote_base_log_folderThis would help future maintainers understand why this normalization exists.
Otherwise ! ✅
|
Thanks for the suggestion! I'm hesitant to make that change right now as I'm not sure if it fits the project norms. I'd prefer to wait and see what a committer thinks first. |
airflow-core/src/airflow/config_templates/airflow_local_settings.py
Outdated
Show resolved
Hide resolved
|
Applied all suggestions, please let me know if anything else needs adjustment. Thanks :) |
|
Thanks for the fix! |
|
Thanks for the suggestion! The bug fix will be recorded in the CHANGELOG, so users should be able to track affected versions from there. No need to worry :) |
* Fix WASB remote logging base path handling * Fix WASB remote logging base path handling by simplifying prefix removal * Clarify handling of WASB remote logging base path format in comments * fix CI static checks error (cherry picked from commit 726d4b5) Co-authored-by: Aaron Chen <nailo2c@gmail.com>
Backport successfully created: v3-1-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
…) (#62456) * Fix WASB remote logging base path handling * Fix WASB remote logging base path handling by simplifying prefix removal * Clarify handling of WASB remote logging base path format in comments * fix CI static checks error (cherry picked from commit 726d4b5) Co-authored-by: Aaron Chen <nailo2c@gmail.com>
Closes: #58946
Why
Following this doc causes Airflow to write logs under an unexpected
<no name>folder in Azure Blob Storage.How
wasbis used as a flag to select the Azure Blob remote logging implementation, but the current code path does not normalize thewasb://scheme.airflow/airflow-core/src/airflow/config_templates/airflow_local_settings.py
Lines 215 to 216 in 89f109b
In the documentation prior to version 12.4.1, users were instructed to use a
wasb-prefix (e.g.wasb-logs). However, PR #51988 notes that this could lead toResourceNotFoundErrorand authentication failures.As a result, the documentation was updated to the current guidance (using
remote_base_log_folder = wasb://...):But the current implementation treats
wasb://as part of the blob name when constructing the remote log path:airflow/providers/microsoft/azure/src/airflow/providers/microsoft/azure/log/wasb_task_handler.py
Line 60 in 89f109b
wasb-logs(works)wasb://logs(problematic)What
Normalize
remote_base_log_folderby stripping thewasb://prefix before constructing the blob key:I intentionally did not change the selection logic to
elif remote_base_log_folder.startswith("wasb://"):because I do not want to impact users who currently configurewasb-...in their settings.airflow/airflow-core/src/airflow/config_templates/airflow_local_settings.py
Line 215 in 89f109b
Result:

Was generative AI tooling used to co-author this PR?