DaanHoogland commented on code in PR #9172:
URL: https://github.com/apache/cloudstack/pull/9172#discussion_r1627133940
##########
server/src/main/java/com/cloud/configuration/Config.java:
##########
@@ -1366,6 +1366,14 @@ public enum Config {
"200",
"The default maximum primary storage space (in GiB) that can be
used for an account",
null),
+DefaultMaxAccountProjects(
Review Comment:
adding the config here is a bit of a deprecated way of doing these. Have a
look at ConfigKey<> for the prefered way.
Define a ConfigKey<> in a manager implementation or a service interface and
then add the configKey to the return of the `getConfirKeys()` in the manager.
You can then use the value more dynamically and it doesn't have to be preloaded
in the config method.
I understand you are just following the convention you see in the files you
are changing but still,...
##########
server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java:
##########
@@ -472,6 +508,51 @@ public void testFindCorrectResourceLimitForDomain() {
Assert.assertEquals(defaultDomainCpuMax, result);
}
+ @Test
Review Comment:
again, can this test case be split?
##########
server/src/main/java/com/cloud/configuration/Config.java:
##########
@@ -1415,6 +1423,7 @@ public enum Config {
DefaultMaxDomainMemory("Domain Defaults", ManagementServer.class,
Long.class, "max.domain.memory", "81920", "The default maximum memory (in MB)
that can be used for a domain", null),
DefaultMaxDomainPrimaryStorage("Domain Defaults", ManagementServer.class,
Long.class, "max.domain.primary.storage", "400", "The default maximum primary
storage space (in GiB) that can be used for a domain", null),
DefaultMaxDomainSecondaryStorage("Domain Defaults",
ManagementServer.class, Long.class, "max.domain.secondary.storage", "800", "The
default maximum secondary storage space (in GiB) that can be used for a
domain", null),
+ DefaultMaxDomainProjects("Domain
Defaults",ManagementServer.class,Long.class,"max.domain.projects","50","The
default maximum number of projects that can be created for a domain",null),
Review Comment:
see above
##########
server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java:
##########
@@ -413,6 +419,36 @@ public void testFindCorrectResourceLimitForAccount() {
Assert.assertEquals(defaultAccountCpuMax, result);
}
+ @Test
Review Comment:
this test has several calls into the code and asserts in between. Do you
think it can be split in separate test cases?
--
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]
