ferruzzi commented on a change in pull request #20819:
URL: https://github.com/apache/airflow/pull/20819#discussion_r782715126



##########
File path: airflow/providers/amazon/aws/operators/eks.py
##########
@@ -89,36 +91,45 @@ class EksCreateClusterOperator(BaseOperator):
          If this is None or empty then the default boto3 behaviour is used.
     :type region: str
 
-    If compute is assigned the value of 'nodegroup', the following are 
required:
+    If compute is assigned the value of 'nodegroup':
 
-    :param nodegroup_name: The unique name to give your Amazon EKS managed 
node group. (templated)
+    :param nodegroup_name: *REQUIRED* The unique name to give your Amazon EKS 
managed node group. (templated)
     :type nodegroup_name: str
-    :param nodegroup_role_arn: The Amazon Resource Name (ARN) of the IAM role 
to associate with the
-         Amazon EKS managed node group. (templated)
+    :param nodegroup_role_arn: *REQUIRED* The Amazon Resource Name (ARN) of 
the IAM role to associate with
+         the Amazon EKS managed node group. (templated)
     :type nodegroup_role_arn: str
+    :param create_nodegroup_kwargs: Optional parameters to pass to the 
CreateNodegroup API (templated)
+    :type: Dict
 
-    If compute is assigned the value of 'fargate', the following are required:
 
-    :param fargate_profile_name: The unique name to give your AWS Fargate 
profile. (templated)
+    If compute is assigned the value of 'fargate':
+
+    :param fargate_profile_name: *REQUIRED* The unique name to give your AWS 
Fargate profile. (templated)
     :type fargate_profile_name: str
-    :param fargate_pod_execution_role_arn: The Amazon Resource Name (ARN) of 
the pod execution role to
-         use for pods that match the selectors in the AWS Fargate profile. 
(templated)
+    :param fargate_pod_execution_role_arn: *REQUIRED* The Amazon Resource Name 
(ARN) of the pod execution
+         role to use for pods that match the selectors in the AWS Fargate 
profile. (templated)
     :type podExecutionRoleArn: str
     :param selectors: The selectors to match for pods to use this AWS Fargate 
profile. (templated)
     :type selectors: List
+    :param create_fargate_profile_kwargs: Optional parameters to pass to the 
CreateFargateProfile API
+         (templated)
+    :type: Dict
 
     """
 
     template_fields: Sequence[str] = (
         "cluster_name",
         "cluster_role_arn",
         "resources_vpc_config",
+        "create_cluster_kwargs",
         "compute",
         "nodegroup_name",
         "nodegroup_role_arn",
+        "create_nodegroup_kwargs",
         "fargate_profile_name",
         "fargate_pod_execution_role_arn",
         "fargate_selectors",
+        "create_fargate_profile_kwargs",

Review comment:
       > what do you think about dropping the create_ so it would juts be 
cluster_kwargs, nodegroup_kwargs, fargate_profile_kwargs?
   
   Could do that.  I left it long like this so we didn't paint ourselves into a 
corner.  I was thinking this way gave flexibility in case other endpoints got 
kwargs later, or maybe we go back and decide to paginate the list operators and 
need a list_clusters_kwargs or something, but that's maybe needless 
future-proofing?  
   
   > i'm also tempted to suggest deprecating non-kwargs params (so in future 
we'd can only use the kwargs params) but maybe people usually only need the 
existing params and it's convenient to have them?
   
   Yeah, that was something I discussed with @o-nikolas.  That or implementing 
a hybrid where you could use them explicitly or within the cluster_kwargs and 
the logic would handle it for you either way, but that made the code 
significantly (and needlessly?) more complicated so we settled on this as the 
easiest non-breaking answer.   What do you think?




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


Reply via email to