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

Fix oss key prefix bug in oss_task_handler, alibaba-provider #39627

Merged
merged 3 commits into from
May 15, 2024

Conversation

EricGao888
Copy link
Member

@EricGao888 EricGao888 commented May 15, 2024


Fix bug discussed in #35654

Sorry for the late fix, I was quite busy at that time when the discussion started. But I think it is better late than never : )

In oss_task_handler of alibaba-provider, oss_write() passes the key with prefix into oss_log_exists(), which duplicately adds the same prefix. This PR fixes the bug and related UT cases.

oss_remote_log_location = f"{self.base_folder}/{remote_log_location}"
pos = 0
if append and self.oss_log_exists(oss_remote_log_location):

def oss_log_exists(self, remote_log_location):
"""
Check if remote_log_location exists in remote storage.
:param remote_log_location: log's location in remote storage
:return: True if location exists else False
"""
oss_remote_log_location = f"{self.base_folder}/{remote_log_location}"

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@EricGao888
Copy link
Member Author

Hi @potiuk , could you please help with the review when available? THX

@potiuk potiuk merged commit ba13d0c into apache:main May 15, 2024
39 checks passed
RNHTTR pushed a commit to RNHTTR/airflow that referenced this pull request Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants