erikbocks commented on PR #14255: URL: https://github.com/apache/cloudstack/pull/14255#issuecomment-6274925099
@rajiv-jain-netapp, thanks for the reviews! I made the following changes to this PR: Implemented null check for offerings, Changed how the backup path is stored in the response map and improved the test cases. For the backup path map, thank you for pointing out about the same volume name replacement scenario. I changed the map to now use the volume UUID as key. Also, I created a new structure for the map value that stores the volume name and its image storage path as attributes. Regarding the usage of storage data for the response, as the only value stored by `internal_backup_view` is the image store ID, obtained by fetching the backup details, I preferred not to include storage data because it would be necessary to include a new join on the image store table. Can you give your opinion on this? For the tests, I extracted the `BackupResponse` related method to a separated class, so we could target the specific test cases for backup responses and reuse existing code for different scenarios. The following cases are addressed in this file: - Default response (the test that already exists); - KBOSS backup response for Admins; - KBOSS backup response for normal users; - KBOSS backup response with multiple equal named volumes; - KBOSS backup response with null offering -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
