aliyun provider oss_task_handler.py has bug about oss_log_exists

[advancehuanghang]

We are using oss as remote log storage, and we found a issue when run with deferrable operator, the root cause is here:

def oss_write(self, log, remote_log_location, append=True) -> bool:
oss_remote_log_location = f"{self.base_folder}/{remote_log_location}"
pos = 0
if append and self.oss_log_exists(oss_remote_log_location):
head = self.hook.head_key(self.bucket_name, oss_remote_log_location)
pos = head.content_length
self.log.info("log write pos is: %s", str(pos))

And then in oss_log_exists function:

def oss_log_exists(self, remote_log_location):
oss_remote_log_location = f"{self.base_folder}/{remote_log_location}"
with contextlib.suppress(Exception):
return self.hook.key_exist(self.bucket_name, oss_remote_log_location)
return False

So the target oss_remote_log_location has a redundant base_folder prefix, and which is not we expected.

Can any one fix this issue ASAP?

thanks.

[advancehuanghang]

@EricGao888 @hussein-awala

[EricGao888]

@advancehuanghang Could u plz open an issue for this instead of posting it in discussion section? I could submit a fix for this : )

[potiuk]

There is no need. You can add PR without having issue. Most of PRs in airflow are done this way. We prefer solutions (especially if they contain also problem description) - rather than having to create separate problem description - we see it as absolutely not needed beaurocracy.

[advancehuanghang]

yes , please fix it go ahead : )

[EricGao888]

Hello @potiuk, thanks for your reminder. I respect the culture in this community and glad to follow. Just to clarify, I was not doing absolutely not needed bureaucracy here, I just hadn't known the rule.

[potiuk]

Cool :) . Same here ... Maybe it should be more prominent.

BTW. I've heard a thing yesterday which is very appropriate

If they wanted to make beaurocracy easier, they would have chosen an easier spelled word.

I really liked it.