-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for autoscale for HDInsight clusters #8104
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kosinsky
Thanks for this PR. It's looking good. We've done some refactor/tidying work that have moved some of the helper code around, so this will need a rebase (it should do so cleanly I think). There's one change I think will need to be made after that (commented below). Once that's done I'll run the CI tests and I think this should be fine as is. There's an additional question on the bug you mention which would be good to have an answer on.
Thanks again.
azurerm/internal/services/hdinsight/tests/hdinsight_hadoop_cluster_resource_test.go
Outdated
Show resolved
Hide resolved
Merge and post merge refactoring reactions are done. Please, take a look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @kosinsky
Thanks for pushing those changes :)
I've taken a look through and left some comments inline but this is looking good - if we can fix up the comments then this should otherwise be good to merge 👍
Thanks!
azurerm/internal/services/hdinsight/tests/hdinsight_hadoop_cluster_resource_test.go
Outdated
Show resolved
Hide resolved
website/docs/r/hdinsight_interactive_query_cluster.html.markdown
Outdated
Show resolved
Hide resolved
Co-authored-by: Tom Harvey <[email protected]>
Co-authored-by: Tom Harvey <[email protected]>
Co-authored-by: Tom Harvey <[email protected]>
Co-authored-by: Tom Harvey <[email protected]>
Co-authored-by: Tom Harvey <[email protected]>
Co-authored-by: Tom Harvey <[email protected]>
Co-authored-by: Tom Harvey <[email protected]>
Co-authored-by: Tom Harvey <[email protected]>
@tombuildsstuff @jackofallops I addressed your feedback. Sorry for delay. |
@kosinsky thank you for all this work! It's much appreciated. In case this was drowned in other notifications and forgotten about I wanted to leave a comment in hope it attracts some attention to a nearly finished PR :) |
@tombuildsstuff @jackofallops Could you take one more look? I addressed your feedback. Please, let me know if I missed something or you have additional one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @kosinsky! I got a chance to look over this and it looks good! The only issue is that it's quite a bit behind...through no fault of your own. Once it's back up to speed, we can get it merged quickly.
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Adds autoscale (capacity and scheduled) support for HDInsight clusters. Following resources were updated :
Others weren't modified, because:
Capacity based autoscale:
Scheduled autoscale:
Tests:
Fixes #6470