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/10/19 19:22:40 UTC
[GitHub] [airflow] vincbeck opened a new pull request, #27149: Fix example_emr_serverless system test
vincbeck opened a new pull request, #27149:
URL: https://github.com/apache/airflow/pull/27149
Fix `example_emr_serverless` system test. EMR does not allow cross region calls. The region was hardcoded for a S3 file to `us-east-1`. Any execution of this system test outside of `us-east-1` results in a failure.
--
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
[GitHub] [airflow] eladkal commented on a diff in pull request #27149: Fix example_emr_serverless system test
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #27149:
URL: https://github.com/apache/airflow/pull/27149#discussion_r1000189443
##########
airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -220,7 +220,7 @@ def check_for_bucket(self, bucket_name: str | None = None) -> bool:
# https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.head_bucket
return_code = int(e.response['Error']['Code'])
if return_code == 404:
- self.log.error('Bucket "%s" does not exist', bucket_name)
+ self.log.info('Bucket "%s" does not exist', bucket_name)
Review Comment:
why do we change log level only for 404?
--
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
[GitHub] [airflow] vincbeck commented on a diff in pull request #27149: Fix example_emr_serverless system test
Posted by GitBox <gi...@apache.org>.
vincbeck commented on code in PR #27149:
URL: https://github.com/apache/airflow/pull/27149#discussion_r1005766058
##########
airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -220,7 +220,7 @@ def check_for_bucket(self, bucket_name: str | None = None) -> bool:
# https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.head_bucket
return_code = int(e.response['Error']['Code'])
if return_code == 404:
- self.log.error('Bucket "%s" does not exist', bucket_name)
+ self.log.info('Bucket "%s" does not exist', bucket_name)
Review Comment:
@eladkal WDYT?
--
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
[GitHub] [airflow] eladkal merged pull request #27149: Fix example_emr_serverless system test
Posted by GitBox <gi...@apache.org>.
eladkal merged PR #27149:
URL: https://github.com/apache/airflow/pull/27149
--
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
[GitHub] [airflow] vincbeck commented on a diff in pull request #27149: Fix example_emr_serverless system test
Posted by GitBox <gi...@apache.org>.
vincbeck commented on code in PR #27149:
URL: https://github.com/apache/airflow/pull/27149#discussion_r1000703520
##########
airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -220,7 +220,7 @@ def check_for_bucket(self, bucket_name: str | None = None) -> bool:
# https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.head_bucket
return_code = int(e.response['Error']['Code'])
if return_code == 404:
- self.log.error('Bucket "%s" does not exist', bucket_name)
+ self.log.info('Bucket "%s" does not exist', bucket_name)
Review Comment:
Well. If it's 403 the bucket exists but the user does not have permissions so I think it makes sense to log an error for that use case
--
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
[GitHub] [airflow] vincbeck commented on a diff in pull request #27149: Fix example_emr_serverless system test
Posted by GitBox <gi...@apache.org>.
vincbeck commented on code in PR #27149:
URL: https://github.com/apache/airflow/pull/27149#discussion_r1000705041
##########
airflow/providers/amazon/aws/hooks/s3.py:
##########
@@ -220,7 +220,7 @@ def check_for_bucket(self, bucket_name: str | None = None) -> bool:
# https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.head_bucket
return_code = int(e.response['Error']['Code'])
if return_code == 404:
- self.log.error('Bucket "%s" does not exist', bucket_name)
+ self.log.info('Bucket "%s" does not exist', bucket_name)
Review Comment:
On the other side, 404 is a very valid use case. If the bucket exists (and user has permissions), we should not log an error for that
--
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