-
Notifications
You must be signed in to change notification settings - Fork 30
[artifact-manager] (#898) Fix push of vROps policies #911
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
base: main
Are you sure you want to change the base?
Conversation
...anager/src/main/java/com/vmware/pscoe/iac/artifact/aria/operations/rest/RestClientVrops.java
Outdated
Show resolved
Hide resolved
| Element policyElement = policyElements.get(i); | ||
| String policyName = policyElement.getAttribute("name"); | ||
| HashMap<String, String> childMap = new HashMap<>(); | ||
| Optional<Policy> currentPolicy = allPolicies.stream().filter(pol -> pol.getName().equals(policyElement.getAttribute("name"))).findAny(); |
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.
What amount of policies do we expect here? I am wondering if it makes sense to convert this to a map and use the name or id for key so we can reduce the filtering operations in case we expect a big list.
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.
What amount of policies do we expect here? I am wondering if it makes sense to convert this to a map and use the name or id for key so we can reduce the filtering operations in case we expect a big list.
From what I have saw they are not that many.
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.
Map also would make sure no duplicate policy ids are present in the zip.
| */ | ||
| private void importPolicies(final Package vropsPackage, final File tmpDir) { | ||
|
|
||
| List<AlertDefinition> alertDefinitions = restClient.getAlltDefinitionsOfType(VropsPackageMemberType.ALERT_DEFINITION) |
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.
Minor thing but there is a "t" beteween All and Definitions in this functions and the ones belows
| ZipOutputStream zos = new ZipOutputStream(fos); | ||
| FileInputStream fis = new FileInputStream(file.getPath())) { | ||
|
|
||
| ZipEntry zipEntry = new ZipEntry(new File(file.getPath()).getName()); |
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.
While do we need to create a new File() here from the file input?
fixed some cosmetic errors
.../artifact-manager/src/main/java/com/vmware/pscoe/iac/artifact/common/utils/XmlUtilities.java
Show resolved
Hide resolved
VenelinBakalov
left a comment
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.
Pending discussion after further discoveries
Description
Fix for failing to push policies for vROps. Implemented functionallity of each policy .xml to check for missing alerts, symptoms and recommendations on the target server.
Checklist
Fixed #XXX -orClosed #XXX -prefix to auto-close the issueTesting
Tested with complex policies on both local and client environment.
Release Notes
Fixes #898