Xikui Wang has posted comments on this change. Change subject: Feed Connection Refactoring ......................................................................
Patch Set 5: (7 comments) I added several comments to yours. I will try to do some cleaning and merge the updates from master as well. Thanks! Sorry for the delay. I was preparing for the candidacy exam preparation. 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 I added two local variables in QueryTranslator before calling getConnectJob. Is that ok? Line 162: PrintWriter writer = new PrintWriter(System.err, true); > MAJOR SonarQube violation: Any idea about this issue? Line 197: ingestionOp = new FeedIntakeOperatorDescriptor(jobSpec, feed, firstOp.getAdaptorLibraryName(), > when will the adapterFactory be null? If it's external adaptor from UDF, this will be null. It will be bind to class at execution time. Line 232: if (opDesc instanceof AsterixLSMTreeInsertDeleteOperatorDescriptor > I believe that we don't need the if or the else here!! The reason I didn't remove feed collect operator from the workflow is for feed policy. I couldn't find a better place to adopt feed policy in current setting, so I kept the feed collect operator.... 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 There is one test case connect-feed added for this case. I will change it to query for more detailed connection info. Line 2143: try { > what if feed has already been started? Fixed this, as well as for stopFeed. I met one tricky case in this though. If I execute 2 stop statement with small time gap, the 2nd stop will throw an exception as the 1st stop is not fully executed yet. Is that an issue? Line 2183: // TODO: check feed is actually running. > do the todo and add a test case? Done with the start fix. -- 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 <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Xikui Wang <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
