Alon Bar-Lev has posted comments on this change.

Change subject: core : Modify MLA to use engine session info
......................................................................


Patch Set 2:

(3 comments)

looks good! very complex for me to understand the sqls I will look at this 
later again, minor notes.

http://gerrit.ovirt.org/#/c/35362/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java:

Line 2308:     protected MacPoolManagerStrategy getMacPool() {
Line 2309:         return 
MacPoolPerDcSingleton.getInstance().poolForDataCenter(getStoragePoolId());
Line 2310:     }
Line 2311: 
Line 2312:     protected long getSessionId() {
can we call this getSessionKey or something to differentiate between session id?
Line 2313:         String sessionId = getParameters().getSessionId();
Line 2314:         if (sessionId == null && getContext() != null && 
getContext().getEngineContext() != null) {
Line 2315:             sessionId = 
getContext().getEngineContext().getSessionId();
Line 2316:         }


Line 2313:         String sessionId = getParameters().getSessionId();
Line 2314:         if (sessionId == null && getContext() != null && 
getContext().getEngineContext() != null) {
Line 2315:             sessionId = 
getContext().getEngineContext().getSessionId();
Line 2316:         }
Line 2317:         return 
SessionDataContainer.getInstance().getSessionId(sessionId);
ok, so this is confusing... but now I follow... should be: getSessionKey as 
well... or getSessionDAOKey or anything that will tell us this is from the 
session table.
Line 2318:     }


http://gerrit.ovirt.org/#/c/35362/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/aaa/SessionDataContainer.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/aaa/SessionDataContainer.java:

Line 100:             engineSession.setUserId(user.getId());
Line 101:             engineSession.setUserName(user.getLoginName());
Line 102:             engineSession.setGroupIds(user.getGroupIds());
Line 103:             long sessionSeqId = 
getDbFacade().getEngineSessionDao().save(engineSession);
Line 104:             sessionToIdMap.put(sessionId, sessionSeqId);
no need for sessionSeqId temp variable.
Line 105:         }
Line 106:     }
Line 107: 
Line 108:     public long getSessionId(String sessionId) {


-- 
To view, visit http://gerrit.ovirt.org/35362
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I28f3853166fe635941e65ea2461f7ecda20f4a2a
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to