asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdullah alamoudi (Code Review)" <>
Subject Change in asterixdb[master]: Feed Connection Refactoring
Date Wed, 16 Nov 2016 00:42:30 GMT
abdullah alamoudi has posted comments on this change.

Change subject: Feed Connection Refactoring

Patch Set 5:


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.

File asterixdb/asterix-active/src/main/java/org/apache/asterix/active/message/

Line 31: 
File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/external/

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

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,
return immediately rather than assign then return

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

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

Line 405:     }
remove unused
File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/

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?
File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/

Line 95: 
get rid of connection status as it is not needed anymore?
File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/

Line 44:     public FeedConnectJobInfo(EntityId entityId, JobId jobId, ActivityState state,
JobSpecification spec) {
rename this class?
File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/operators/

this class should go away
File asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj:

do the same changes in the sqlpp.jj file?
File asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj:

Line 1150: }
stop is missing?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic36267eb9a10df21734ce1cc1f38583e23c9e8f0
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Xikui Wang <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: abdullah alamoudi <>
Gerrit-HasComments: Yes

View raw message