You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/03/10 17:18:20 UTC
[GitHub] [airflow] dstandish commented on a change in pull request #22105: Add provide_bucket_name to S3Hook.delete_objects
dstandish commented on a change in pull request #22105:
URL: https://github.com/apache/airflow/pull/22105#discussion_r823324733
##########
File path: airflow/providers/amazon/aws/hooks/s3.py
##########
@@ -145,6 +150,14 @@ def parse_s3_url(s3url: str) -> Tuple[str, str]:
return bucket_name, key
+ @cached_property
+ def bucket_name(self) -> Optional[str]:
+ if self.aws_conn_id:
+ connection = self.get_connection(self.aws_conn_id)
Review comment:
instead fo caching bucket name here, let's cache `airflow_connection` on AwsBaseHook
##########
File path: airflow/providers/amazon/aws/hooks/s3.py
##########
@@ -698,14 +711,20 @@ def delete_bucket(self, bucket_name: str, force_delete: bool = False) -> None:
if force_delete:
bucket_keys = self.list_keys(bucket_name=bucket_name)
if bucket_keys:
- self.delete_objects(bucket=bucket_name, keys=bucket_keys)
+ self.delete_objects(bucket_name=bucket_name, keys=bucket_keys)
self.conn.delete_bucket(Bucket=bucket_name)
- def delete_objects(self, bucket: str, keys: Union[str, list]) -> None:
+ @provide_bucket_name
+ def delete_objects(
+ self,
+ bucket_name: Optional[str] = None,
+ keys: Optional[Union[str, list]] = None,
+ bucket: Optional[str] = None,
Review comment:
i do not think you can reorder the params here since they are no kw-only
##########
File path: airflow/providers/amazon/aws/hooks/s3.py
##########
@@ -145,6 +150,14 @@ def parse_s3_url(s3url: str) -> Tuple[str, str]:
return bucket_name, key
+ @cached_property
+ def bucket_name(self) -> Optional[str]:
+ if self.aws_conn_id:
+ connection = self.get_connection(self.aws_conn_id)
Review comment:
instead fo caching bucket name here, let's cache `airflow_connection` on AwsBaseHook
##########
File path: airflow/providers/amazon/aws/hooks/s3.py
##########
@@ -698,14 +711,20 @@ def delete_bucket(self, bucket_name: str, force_delete: bool = False) -> None:
if force_delete:
bucket_keys = self.list_keys(bucket_name=bucket_name)
if bucket_keys:
- self.delete_objects(bucket=bucket_name, keys=bucket_keys)
+ self.delete_objects(bucket_name=bucket_name, keys=bucket_keys)
self.conn.delete_bucket(Bucket=bucket_name)
- def delete_objects(self, bucket: str, keys: Union[str, list]) -> None:
+ @provide_bucket_name
+ def delete_objects(
+ self,
+ bucket_name: Optional[str] = None,
+ keys: Optional[Union[str, list]] = None,
+ bucket: Optional[str] = None,
Review comment:
i do not think you can reorder the params here since they are no kw-only
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org