Skip to content
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.

Sign up for GitHub

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

Jump to bottom

update instance interface and security group attachment procces #5

Merged
merged 2 commits into from
Feb 20, 2023
Merged

update instance interface and security group attachment procces #5

merged 2 commits into from
Feb 20, 2023

Conversation

anaxaim
Copy link
Contributor

@anaxaim anaxaim commented Feb 19, 2023

  1. Refactored HasChange("interface") then update the instance. There are now 3 cases:
  • Same number of interfaces
  • New interfaces > old interfaces
  • Old interfaces > new interfaces
  1. Changed the security group attachment/detachment for HasChange("interface")
  2. Moved helper functions from "*_instance.go" file to "utils.go" and left only CRUD operations
  3. Added more helper functions for instance cases.

@anaxaim
update instance interface
8dc8860 
1. Refactored HasChange("interface") then update the instance. There are now 3 cases:
- Same number of interfaces
- New interfaces > old interfaces
- Old interfaces > new interfaces
2. Changed the security group attachment/detachment for HasChange("interface")
3. Moved helper functions from "*_instance.go" file to "utils.go" and left only CRUD operations
4. Added more helper functions for instance cases.
@anaxaim anaxaim requested a review from alex2304 February 19, 2023 12:12
alex2304
alex2304 approved these changes Feb 20, 2023
View reviewed changes
Copy link
Collaborator

@alex2304 alex2304 left a comment

Choose a reason for hiding this comment

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

There's a little bit overhead (repeated parts of code inside the condition if d.HasChange("interface"))

But overall it's clear and good solution

Changes are not obligatory, if you agree with them - please apply them before merging.

edgecenter/utils.go Outdated Show resolved Hide resolved
edgecenter/utils.go Outdated Show resolved Hide resolved
edgecenter/utils.go Outdated Show resolved Hide resolved
edgecenter/utils.go Show resolved Hide resolved
edgecenter/resource_edgecenter_instance.go Outdated Show resolved Hide resolved
@anaxaim anaxaim merged commit 65cb0eb into master Feb 20, 2023
@anaxaim anaxaim deleted the CLOUD-174-update_instance_intarface branch February 20, 2023 14:24
Uiopas pushed a commit that referenced this pull request Mar 6, 2024
# This is a combination of 9 commits.
7fe10e5 
# This is the 1st commit message:

feat(CLOUDDEV-568): added new dataSource snapshot

# This is the commit message #2:

feat(CLOUDDEV-568): fix linter errors

# This is the commit message #3:

feat(CLOUDDEV-568): fix errors if volumeid not exist
# This is the commit message #4:

feat(CLOUDDEV-575): changed docs

# This is the commit message #5:

feat(CLOUDDEV-575): added docs for snapshot data source resource

# This is the commit message #6:

feat(CLOUDDEV-568): fix commented errors

# This is the commit message #7:

feat(CLOUDDEV-568): fix commented errors

# This is the commit message #8:

feat(CLOUDDEV-568): fix errors

# This is the commit message #9:

feat(CLOUDDEV-568): fix errors
Uiopas pushed a commit that referenced this pull request Mar 7, 2024
# This is a combination of 5 commits.
10d1acc 
# This is the 1st commit message:

fix(CLOUDDEV-574): up version go client to v2.0.22

# This is the commit message #2:

feat(CLOUDDEV-568): added file for test data source snapshot

# This is the commit message #3:

feat(CLOUDDEV-568): added file for test data source snapshot

# This is the commit message #4:

feat(CLOUDDEV-575): merge resource from Clouddev-568

# This is the commit message #5:

feat(CLOUDDEV-575): added uniq name for volume
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alex2304 alex2304 alex2304 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@anaxaim @alex2304