[GitHub] [airflow] pankajkoti commented on a diff in pull request #31276: Fix missing aws default

["pankajkoti (via GitHub)" <gi...@apache.org>]

pankajkoti commented on code in PR #31276:
URL: https://github.com/apache/airflow/pull/31276#discussion_r1193347936


##########
airflow/providers/amazon/aws/operators/batch.py:
##########
@@ -138,7 +138,7 @@ def __init__(
         waiters: Any | None = None,
         max_retries: int | None = None,
         status_retries: int | None = None,
-        aws_conn_id: str | None = None,
+        aws_conn_id: str | None = 'aws_default',

Review Comment:
   I am going through the operators we have in `amazon/aws` provider. In some places we just have a type annotation saying it is a `str` value. While other places have `str | None` with default `aws_default`. Do we want to continue accepting `None` values and fallback to `boto3` looking for the credentials in the `~/aws/credentials` file in general? In that case, should we update the type annotation to `str | None` in all the operators? If we do not want to fallback and want to mandate an Airflow connection to be used, should we raise an exception when the `aws_conn_id` value is not set?
   
   Additionally, I also see that some operators have `aws_conn_id` in their `template_fields` but not all. Do we need to unify this experience across all operators?



-- 
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