abdullah alamoudi has posted comments on this change.

Change subject: Remove static cc application context instance
......................................................................


Patch Set 5:

(7 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1606/5/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java:

PS5, Line 173:             return new Pair<>(false, expr);
> I see some rebase conflicts in  your future
I have been rebasing quite a lot recently. It is not a problem :p


https://asterix-gerrit.ics.uci.edu/#/c/1606/5/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/replication/NoFaultToleranceStrategy.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/replication/NoFaultToleranceStrategy.java:

Line 58:     private ICCServiceContext serviceCtx;
> MAJOR SonarQube violation:
Done


https://asterix-gerrit.ics.uci.edu/#/c/1606/5/asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/CCApplication.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/CCApplication.java:

PS5, Line 289: appCtx
> should we make this the last parameter to align with all of the other servl
yes we should


https://asterix-gerrit.ics.uci.edu/#/c/1606/5/asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/MessagingChannelInterfaceFactory.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/MessagingChannelInterfaceFactory.java:

PS5, Line 111: 
             :                         
> does the formatter really put a newline here?
this is probably because I changed it to format only changed lines. and so it 
didn't change the following line. There is no winning with this formatting 
thing no matter what setting I choose :\


https://asterix-gerrit.ics.uci.edu/#/c/1606/5/asterixdb/asterix-app/src/main/java/org/apache/asterix/utils/FeedOperations.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/utils/FeedOperations.java:

Line 276:             for (Entry<ConnectorDescriptorId, 
Pair<Pair<IOperatorDescriptor, Integer>, Pair<IOperatorDescriptor, Integer>>> 
entry : subJob
> +1
Done


https://asterix-gerrit.ics.uci.edu/#/c/1606/5/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalScalarFunctionEvaluatorFactory.java
File 
asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalScalarFunctionEvaluatorFactory.java:

PS5, Line 46:                 appCtx == null
            :                         ? (IApplicationContext) 
ctx.getJobletContext().getServiceContext().getApplicationContext()
            :                         : appCtx);
            :  
> ctx.getJobletContext().getServiceContext().getApplicationContext() defined 
we can't.

this was designed (not by me) to run on both cc during constant folding rule 
and on the NC during runtime. In CC, ctx, will be null, in NC, appCtx will be 
null.

It is what it is for now.


https://asterix-gerrit.ics.uci.edu/#/c/1606/5/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/CcApplicationContext.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/CcApplicationContext.java:

PS5, Line 107:        
Logger.getLogger("org.apache.asterix").setLevel(externalProperties.getLogLevel());
             :         
Logger.getLogger("org.apache.hyracks").setLevel(externalProperties.getLogLevel());
> why was this introduced here?
I am not sure. maybe it was here at some point and we lost it with rebasing or 
something.

In short, I have no idea but will remove it unless I remember.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1606
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2e250405967ec880e7af6387aa981f39b3392c0
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Michael Blow <[email protected]>
Gerrit-Reviewer: Steven Jacobs <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to