asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xikui Wang (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Feed Connection Refactoring
Date Wed, 07 Dec 2016 00:15:55 GMT
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 <xkkwww@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Xikui Wang <xkkwww@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message