Alon Bar-Lev has posted comments on this change. Change subject: aaa: Introducing built-in extensions module ......................................................................
Patch Set 4: (8 comments) http://gerrit.ovirt.org/#/c/25253/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeUserPasswordCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeUserPasswordCommand.java: Line 4: import java.util.List; Line 5: Line 6: import org.ovirt.engine.core.extensions.aaa.builtin.ldap.AdActionType; Line 7: import org.ovirt.engine.core.extensions.aaa.builtin.ldap.LdapChangeUserPasswordParameters; Line 8: import org.ovirt.engine.core.extensions.aaa.builtin.ldap.LdapFactory; nothing should import builtin. Line 9: import org.ovirt.engine.core.common.AuditLogType; Line 10: import org.ovirt.engine.core.bll.utils.PermissionSubject; Line 11: import org.ovirt.engine.core.common.action.ChangeUserPasswordParameters; Line 12: http://gerrit.ovirt.org/#/c/25253/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java: Line 14: import org.ovirt.engine.core.extensions.aaa.builtin.ldap.LdapBroker; Line 15: import org.ovirt.engine.core.extensions.aaa.builtin.ldap.LdapFactory; Line 16: import org.ovirt.engine.core.extensions.aaa.builtin.ldap.LdapQueryData; Line 17: import org.ovirt.engine.core.extensions.aaa.builtin.ldap.LdapQueryDataImpl; Line 18: import org.ovirt.engine.core.extensions.aaa.builtin.ldap.LdapQueryType; nothing should import builtin. Line 19: import org.ovirt.engine.core.bll.quota.QuotaManager; Line 20: import org.ovirt.engine.core.common.businessentities.AuditLog; Line 21: import org.ovirt.engine.core.common.businessentities.DbGroup; Line 22: import org.ovirt.engine.core.common.businessentities.DbUser; http://gerrit.ovirt.org/#/c/25253/4/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/adbroker/LdapFilterSearchEnginePreProcessorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/adbroker/LdapFilterSearchEnginePreProcessorTest.java: Line 3: import static org.junit.Assert.assertEquals; Line 4: Line 5: import org.junit.Test; Line 6: import org.ovirt.engine.core.extensions.aaa.builtin.ldap.NoOpLdapFilterSearchEnginePreProcessor; Line 7: import org.ovirt.engine.core.extensions.aaa.builtin.ldap.UpnSplitterLdapFilterSearchEnginePreProcessor; this test should be moved to builtin module or removed. Line 8: Line 9: public class LdapFilterSearchEnginePreProcessorTest { Line 10: Line 11: @Test http://gerrit.ovirt.org/#/c/25253/4/backend/manager/modules/builtin-extensions/pom.xml File backend/manager/modules/builtin-extensions/pom.xml: Line 40: <version>${org.ovirt.engine.api.ovirt-engine-extensions-api.version}</version> Line 41: </dependency> Line 42: <dependency> Line 43: <groupId>org.ovirt.engine.core</groupId> Line 44: <artifactId>aaa</artifactId> this is duplicated with above, no? Line 45: <version>${engine.version}</version> Line 46: </dependency> Line 47: Line 48: <dependency> Line 46: </dependency> Line 47: Line 48: <dependency> Line 49: <groupId>org.slf4j</groupId> Line 50: <artifactId>slf4j-jdk14</artifactId> are you sure you need this? Line 51: </dependency> Line 52: </dependencies> Line 53: <build> Line 54: <plugins> http://gerrit.ovirt.org/#/c/25253/4/backend/manager/modules/builtin-extensions/src/main/resources/META-INF/services/org.ovirt.engine.api.extensions.Extension File backend/manager/modules/builtin-extensions/src/main/resources/META-INF/services/org.ovirt.engine.api.extensions.Extension: Line 1: org.ovirt.engine.core.extensions.aaa.builtin.internal.InternalAuthenticator Line 2: org.ovirt.engine.core.extensions.aaa.builtin.internal.InternalDirectory Line 3: org.ovirt.engine.core.extensions.aaa.builtin.ldap.ProvisionalDirectory Line 4: org.ovirt.engine.core.extensions.aaa.builtin.ldap.ProvisionalAuthenticator can we rename these to something more clear? they are going to stay forever. http://gerrit.ovirt.org/#/c/25253/4/backend/manager/modules/extension-manager/src/main/modules/org/ovirt/engine/core/extension-manager/main/module.xml File backend/manager/modules/extension-manager/src/main/modules/org/ovirt/engine/core/extension-manager/main/module.xml: Line 12: <module name="org.apache.commons.lang"/> Line 13: <module name="org.jboss.modules"/> Line 14: <module name="org.ovirt.engine.core.utils"/> Line 15: <module name="org.ovirt.engine.api.ovirt-engine-extensions-api"/> Line 16: <module name="org.ovirt.engine.core.builtin-extensions"/> why? Line 17: <module name="org.slf4j"/> Line 18: </dependencies> Line 19: http://gerrit.ovirt.org/#/c/25253/4/backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/Extension.java File backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/Extension.java: Line 17: HOME, Line 18: EXTENSION_NAME, Line 19: AAA_CHANGE_EXPIRED_PASSWORD_URL, Line 20: AAA_CHANGE_EXPIRED_PASSWORD_MSG, Line 21: AAA_AUTHENTICATION_CAPABILITIES so we had error in previous build? fix it in own patch and push ASAP. Line 22: }; Line 23: Line 24: public static final int AAA_AUTH_CAP_FLAGS_PASSWORD = (1 << 0); Line 25: public static final int AAA_AUTH_CAP_FLAGS_NEGOTIATING = (1 << 1); -- To view, visit http://gerrit.ovirt.org/25253 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If7ea408b46548ffdad1bf56fb8ab54b68722a480 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
