Change in asterixdb[master]: Feed Connection Refactoring

["abdullah alamoudi (Code Review)" <do...@asterixdb.incubator.apache.org>]

abdullah alamoudi has posted comments on this change.

Change subject: Feed Connection Refactoring
......................................................................


Patch Set 5:

(17 comments)

Looks good overall and I can clearly see that you understand what is happening. Please go over the comments. I will look at the new patch.

Cheers,
Abdullah.

https://asterix-gerrit.ics.uci.edu/#/c/1259/5/asterixdb/asterix-active/src/main/java/org/apache/asterix/active/message/ActiveManagerMessage.java
File asterixdb/asterix-active/src/main/java/org/apache/asterix/active/message/ActiveManagerMessage.java:

Line 31: 
revert?


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

Line 116:     private static final ILangCompilationProvider compilationProvider = new AqlCompilationProvider();
get rid of static compilationProvider
and qtFactory and instead pass them as parameters!


Line 197:             ingestionOp = new FeedIntakeOperatorDescriptor(jobSpec, feed, firstOp.getAdaptorLibraryName(),
when will the adapterFactory be null?


Line 232:                 if (opDesc instanceof AsterixLSMTreeInsertDeleteOperatorDescriptor
I believe that we don't need the if or the else here!!

ideally, we should get rid of the feed collect (you changed it to take input) but it doesn't do anything essentially. we don't need to do this now. we can create an issue to get rid of it.


Line 370:         JobSpecification feedJob;
remove the feedJob variable?
move the declaration of ingestionLocations down to the assignment statement?


Line 386:         feedJob = combineIntakeCollectJobs(metadataProvider, feed, intakeJob, jobsList, feedConnections,
return immediately rather than assign then return


Line 391:     private static Pair<IOperatorDescriptor, AlgebricksPartitionConstraint> buildSendFeedMessageRuntime(
remove unused


Line 400: 
remove unused... and remove the feed message operator completely


Line 405:     }
remove unused


https://asterix-gerrit.ics.uci.edu/#/c/1259/5/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java
File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java:

Line 345:                     case Statement.Kind.CONNECT_FEED:
add test cases that queries the connection dataset after connect and after disconnect?


Line 2143:         try {
what if feed has already been started?
fix and add a test case please :)


Line 2183:             // TODO: check feed is actually running.
do the todo and add a test case?


https://asterix-gerrit.ics.uci.edu/#/c/1259/5/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedConnectionRequest.java
File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedConnectionRequest.java:

Line 95: 
get rid of connection status as it is not needed anymore?


https://asterix-gerrit.ics.uci.edu/#/c/1259/5/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/FeedConnectJobInfo.java
File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/FeedConnectJobInfo.java:

Line 44:     public FeedConnectJobInfo(EntityId entityId, JobId jobId, ActivityState state, JobSpecification spec) {
rename this class?


https://asterix-gerrit.ics.uci.edu/#/c/1259/5/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedMessageOperatorDescriptor.java
File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedMessageOperatorDescriptor.java:

this class should go away


https://asterix-gerrit.ics.uci.edu/#/c/1259/5/asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj
File asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj:

do the same changes in the sqlpp.jj file?


https://asterix-gerrit.ics.uci.edu/#/c/1259/5/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
File asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj:

Line 1150: }
stop is missing?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic36267eb9a10df21734ce1cc1f38583e23c9e8f0
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <xk...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes