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 2021/11/17 10:42:58 UTC
[GitHub] [airflow] andormarkus opened a new issue #19641: EKSCreateNodegroupOperator - extra kwargs
andormarkus opened a new issue #19641:
URL: https://github.com/apache/airflow/issues/19641
### Description
Boto3 / eks / create_nodegroup api supports the following kwargs:
```python
clusterName='string',
nodegroupName='string',
scalingConfig={
'minSize': 123,
'maxSize': 123,
'desiredSize': 123
},
diskSize=123,
subnets=[
'string',
],
instanceTypes=[
'string',
],
amiType='AL2_x86_64'|'AL2_x86_64_GPU'|'AL2_ARM_64'|'CUSTOM'|'BOTTLEROCKET_ARM_64'|'BOTTLEROCKET_x86_64',
remoteAccess={
'ec2SshKey': 'string',
'sourceSecurityGroups': [
'string',
]
},
nodeRole='string',
labels={
'string': 'string'
},
taints=[
{
'key': 'string',
'value': 'string',
'effect': 'NO_SCHEDULE'|'NO_EXECUTE'|'PREFER_NO_SCHEDULE'
},
],
tags={
'string': 'string'
},
clientRequestToken='string',
launchTemplate={
'name': 'string',
'version': 'string',
'id': 'string'
},
updateConfig={
'maxUnavailable': 123,
'maxUnavailablePercentage': 123
},
capacityType='ON_DEMAND'|'SPOT',
version='string',
releaseVersion='string'
```
The current implementation of the operator support the following kwargs only:
```python
clusterName='string',
nodegroupName='string',
subnets=[ 'string', ],
nodeRole='string',
```
### Use case/motivation
With the current implementation of the Operator you can bring up basic EKS nodegroup which is not useful. I want to fully configure my nodegroup with this operation like I can do it boto3.
### Related issues
Continue of #19372
### Are you willing to submit a PR?
- [ ] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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] ferruzzi edited a comment on issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
ferruzzi edited a comment on issue #19641:
URL: https://github.com/apache/airflow/issues/19641#issuecomment-978658741
How strongly do you feel about that? Leaving them explicitly called out like that makes it obvious they are mandatory and enforces that, which (maybe?) makes it easier for new users to get started. I kind of like that. Would it be a deal-breaker this way?
I started implementing it and realized that I have an issue somewhere with my environment I need to sort out. For whatever reason, I can't create nodegroups at all right now.
--
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] andormarkus commented on issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
andormarkus commented on issue #19641:
URL: https://github.com/apache/airflow/issues/19641#issuecomment-971454118
Hi @ferruzzi #19372 continues here
--
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] kaxil closed issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
kaxil closed issue #19641:
URL: https://github.com/apache/airflow/issues/19641
--
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] andormarkus commented on issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
andormarkus commented on issue #19641:
URL: https://github.com/apache/airflow/issues/19641#issuecomment-972180252
For me the best solution would be if I could pass the for mandatory parameters in the `create_nodegroup_kwargs` as well.
--
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] ferruzzi edited a comment on issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
ferruzzi edited a comment on issue #19641:
URL: https://github.com/apache/airflow/issues/19641#issuecomment-978658741
How strongly do you feel about that? Leaving them explicitly called out like that makes it obvious they are mandatory and enforces that, which (maybe?) makes it easier for new users to get started. I kind of like that. Would it be a deal-breaker this way?
I started implementing it and realized that there is an issue with the current version of the Operator in the main branch; going to have to sort that out first. Which version of airflow/operators are you using? That should help me sort out what/where it went wrong. With the current main branch, I can create a cluster, but creating a nodegroup fails to attach to the cluster. I presume you aren't seeing that on whichever version you are on?
--
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] ferruzzi commented on issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on issue #19641:
URL: https://github.com/apache/airflow/issues/19641#issuecomment-978658741
How strongly do you feel about that? Leaving them explicitly called out like that makes it obvious they are mandatory and enforces that, which (maybe?) makes it easier for new users to get started. I kind of like that. Would it be a deal-breaker this way?
I started implementing it and realized that there is an issue with the current version of the Operator in the main branch; going to have to sort that out first,. Which version if airflow/operators are you using? That should help me sort out what/where it went wrong. With the current main branch, I can create a cluster, but creating a nodegroup fails to attach to the cluster. I presume you aren't seeing that on whichever version you are on?
--
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] ferruzzi commented on issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on issue #19641:
URL: https://github.com/apache/airflow/issues/19641#issuecomment-971874324
@andormarkus (and anyone else who wants to weigh in, of course) - It looks like the way Glue solved this was by adding a dict called "create_job_kwargs" which gets unpacked in the boto call. That would keep the code concise and allow for future boto changes easily, but makes that extra step of packing it into a dict for the end user. The alternative is defining every potential parameter that boto accepts, and having to keep that up to date. So this dict is cleaner in the code and a bit more future-proof perhaps, but an extra step for the user. How do you feel about this?
Example usage should look something like:
```
create_nodegroup = EKSCreateNodegroupOperator(
task_id='create_eks_nodegroup',
cluster_name=cluster_name,
nodegroup_name=node_group_name,
nodegroup_subnets=get_subnets(),
nodegroup_role_arn=get_node_role(),
create_nodegroup_kwargs={
amiType = "BOTTLEROCKET_x86_64",
instanceTypes = "t3.large",
capacityType = "ON_DEMAND"
}
)
```
I'd add the same workaround for create_cluster and create_fargate_profile while I'm at it.
--
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] andormarkus edited a comment on issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
andormarkus edited a comment on issue #19641:
URL: https://github.com/apache/airflow/issues/19641#issuecomment-972180252
For me the best solution would be if I could pass the four mandatory parameters in the `create_nodegroup_kwargs` as well.
--
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] potiuk commented on issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19641:
URL: https://github.com/apache/airflow/issues/19641#issuecomment-971911653
LGTM
--
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] ferruzzi commented on issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on issue #19641:
URL: https://github.com/apache/airflow/issues/19641#issuecomment-993113732
How do we feel about this implementation? The required parameters are still there in named args, but if they are not passed, then it checks the operator_kwargs for their values.
https://github.com/ferruzzi/airflow/tree/kwargs-kpo
--
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] ferruzzi edited a comment on issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
ferruzzi edited a comment on issue #19641:
URL: https://github.com/apache/airflow/issues/19641#issuecomment-993113732
How do we feel about this implementation? The required parameters are still there in named args, but if they are not passed, then it checks the operator_kwargs for their values. So all three of these are valid:
Pass only the required values by name:
```
create_nodegroup = EKSCreateNodegroupOperator(
task_id='create_eks_nodegroup',
cluster_name=cluster_name,
nodegroup_name=node_group_name,
nodegroup_subnets=get_subnets(),
nodegroup_role_arn=get_node_role(),
)
```
Pass the required values by name and the optional values in an operator_kwargs
```
create_nodegroup = EKSCreateNodegroupOperator(
task_id='create_eks_nodegroup',
cluster_name=cluster_name,
nodegroup_name=node_group_name,
nodegroup_subnets=get_subnets(),
nodegroup_role_arn=get_node_role(),
create_nodegroup_kwargs={
amiType = "BOTTLEROCKET_x86_64",
instanceTypes = "t3.large",
capacityType = "ON_DEMAND"
}
)
```
Pass everything in as operator_kwargs:
```
create_nodegroup = EKSCreateNodegroupOperator(
task_id='create_eks_nodegroup',
create_nodegroup_kwargs={
cluster_name=cluster_name,
nodegroup_name=node_group_name,
nodegroup_subnets=get_subnets(),
nodegroup_role_arn=get_node_role(),
amiType = "BOTTLEROCKET_x86_64",
instanceTypes = "t3.large",
capacityType = "ON_DEMAND"
}
)
```
https://github.com/ferruzzi/airflow/tree/kwargs-kpo
--
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] ferruzzi edited a comment on issue #19641: EKSCreateNodegroupOperator - missing kwargs
Posted by GitBox <gi...@apache.org>.
ferruzzi edited a comment on issue #19641:
URL: https://github.com/apache/airflow/issues/19641#issuecomment-993113732
How do we feel about [this implementation](https://github.com/ferruzzi/airflow/tree/kwargs-kpo)? The required parameters are still there in named args, but if they are not passed, then it checks the operator_kwargs for their values. So all three of these are valid:
Pass only the required values by name:
```
create_nodegroup = EKSCreateNodegroupOperator(
task_id='create_eks_nodegroup',
cluster_name=cluster_name,
nodegroup_name=node_group_name,
nodegroup_subnets=get_subnets(),
nodegroup_role_arn=get_node_role(),
)
```
Pass the required values by name and the optional values in an operator_kwargs
```
create_nodegroup = EKSCreateNodegroupOperator(
task_id='create_eks_nodegroup',
cluster_name=cluster_name,
nodegroup_name=node_group_name,
nodegroup_subnets=get_subnets(),
nodegroup_role_arn=get_node_role(),
create_nodegroup_kwargs={
amiType = "BOTTLEROCKET_x86_64",
instanceTypes = "t3.large",
capacityType = "ON_DEMAND"
}
)
```
Pass everything in as operator_kwargs:
```
create_nodegroup = EKSCreateNodegroupOperator(
task_id='create_eks_nodegroup',
create_nodegroup_kwargs={
cluster_name=cluster_name,
nodegroup_name=node_group_name,
nodegroup_subnets=get_subnets(),
nodegroup_role_arn=get_node_role(),
amiType = "BOTTLEROCKET_x86_64",
instanceTypes = "t3.large",
capacityType = "ON_DEMAND"
}
)
```
--
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