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 <xkk...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes