Skip to content

Adding support for VMLimit in AzureVMTemplateBuilder #652

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alwaysharsha
Copy link

Testing done

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@alwaysharsha alwaysharsha force-pushed the hs-vmtemplate-maxlimit branch from 629a9fa to 9ffd059 Compare April 8, 2025 13:48
@alwaysharsha alwaysharsha force-pushed the hs-vmtemplate-maxlimit branch from 9a94207 to d50a471 Compare April 8, 2025 14:22
@@ -96,7 +98,7 @@ public AzureVMAgentTemplate build() {
fluent.getDescription(),
fluent.getLabels(),
fluent.getLocation(),
null,
new NoAvailabilityRequired(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change here?


public class AzureVMTemplateBuilder extends AzureVMTemplateFluent<AzureVMTemplateBuilder> {

private AzureVMTemplateFluent<?> fluent;

public AzureVMTemplateBuilder() {
this.fluent = this;
fluent.withMaxVirtualMachinesLimit("10");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't right it shouldn't be here


public class AzureVMTemplateBuilder extends AzureVMTemplateFluent<AzureVMTemplateBuilder> {

private AzureVMTemplateFluent<?> fluent;

public AzureVMTemplateBuilder() {
this.fluent = this;
fluent.withMaxVirtualMachinesLimit("10");
}

public AzureVMTemplateBuilder(AzureVMAgentTemplate template) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you also need to call setMaxVirtualMachinesLimit in the build method further down

@@ -57,6 +59,7 @@ public class AzureVMTemplateFluent<T extends AzureVMTemplateFluent<T>> {
private List<AzureTagPair> cloudTags;

public AzureVMTemplateFluent() {
maxVirtualMachinesLimit = "10";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
maxVirtualMachinesLimit = "10";
maxVirtualMachinesLimit = 10;

@@ -12,6 +12,8 @@

public class AzureVMTemplateFluent<T extends AzureVMTemplateFluent<T>> {

private String maxVirtualMachinesLimit;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private String maxVirtualMachinesLimit;
private int maxVirtualMachinesLimit;

@@ -73,6 +76,11 @@ public AzureVMTemplateFluent() {
}

//CHECKSTYLE:OFF
public T withMaxVirtualMachinesLimit(String maxVirtualMachinesLimit) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public T withMaxVirtualMachinesLimit(String maxVirtualMachinesLimit) {
public T withMaxVirtualMachinesLimit(int maxVirtualMachinesLimit) {

@@ -208,6 +216,10 @@ public T withAdminCredential(String credentialsId) {
}
//CHECKSTYLE:ON

public String getMaxVirtualMachinesLimit() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public String getMaxVirtualMachinesLimit() {
public int getMaxVirtualMachinesLimit() {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants