shahar1 commented on code in PR #67076:
URL: https://github.com/apache/airflow/pull/67076#discussion_r3287141396
##########
providers/google/src/airflow/providers/google/marketing_platform/operators/bid_manager.py:
##########
@@ -338,19 +340,25 @@ def redirect_request(self, req, fp, code, msg, headers,
newurl):
no_redirect_opener = urllib.request.build_opener(_NoRedirect)
self.log.info("Starting downloading report %s", self.report_id)
with tempfile.NamedTemporaryFile(delete=False) as temp_file:
- with no_redirect_opener.open(file_url) as response: # nosec
- shutil.copyfileobj(response, temp_file, length=self.chunk_size)
-
- temp_file.flush()
- # Upload the local file to bucket
- bucket_name = self._set_bucket_name(self.bucket_name)
- gcs_hook.upload(
- bucket_name=bucket_name,
- object_name=report_name,
- gzip=self.gzip,
- filename=temp_file.name,
- mime_type="text/csv",
- )
+ try:
+ with no_redirect_opener.open(file_url) as response: # nosec
+ shutil.copyfileobj(response, temp_file,
length=self.chunk_size)
+
+ temp_file.flush()
+ # Upload the local file to bucket
+ bucket_name = self._set_bucket_name(self.bucket_name)
+ gcs_hook.upload(
+ bucket_name=bucket_name,
+ object_name=report_name,
+ gzip=self.gzip,
+ filename=temp_file.name,
+ mime_type="text/csv",
+ )
+ finally:
+ # Always remove the unencrypted report left in the temp
directory,
+ # on both the success and the failure path.
+ with contextlib.suppress(OSError):
+ os.unlink(temp_file.name)
Review Comment:
Here you unlink a file with the same context manager that created it. On
some operation systems it might raise a `PermissionError`, which
`contextlib.suppress(OSError)` swallows in turn - and then we're having the
same issue which you tried to resolve.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
