Till Westmann has posted comments on this change. Change subject: Add flush() to IFrameWriter ......................................................................
Patch Set 9: (17 comments) https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-app/src/main/java/org/apache/asterix/feed/FeedMessageReceiver.java File asterix-app/src/main/java/org/apache/asterix/feed/FeedMessageReceiver.java: Line 95: } What does this do? https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplicationEntryPoint.java File asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplicationEntryPoint.java: Line 273: //Clean any temporary files Revert the file? https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-common/src/main/java/org/apache/asterix/common/config/AsterixPropertiesAccessor.java File asterix-common/src/main/java/org/apache/asterix/common/config/AsterixPropertiesAccessor.java: Line 130: Revert the file? https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-external-data/pom.xml File asterix-external-data/pom.xml: Line 279: </dependency> > Are these MIT licensed? And do the POMs have the correct metadata about the It seems that core-io is indeed MIT-licensed and rxjava (on GitHub) seems to be Apache-licensed with a copyright: Copyright 2012 Netflix, Inc. https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-external-data/src/main/java/org/apache/asterix/external/api/IRawRecord.java File asterix-external-data/src/main/java/org/apache/asterix/external/api/IRawRecord.java: Line 40: public Class<?> getRecordClass(); It seems that this is never used inside the code base. Do you know why we need it? Line 56: public void setRecord(T t); The getter is only called "get", should we call this one "set"? https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-external-data/src/main/java/org/apache/asterix/external/api/IRecordReader.java File asterix-external-data/src/main/java/org/apache/asterix/external/api/IRecordReader.java: Line 61: public Class<? extends T> getRecordClass() throws IOException; This seems to be unused in our code. Do you know why we need it? Line 73: public default void setController(AbstractFeedDataFlowController controller) throws UnsupportedOperationException { Why is this not IDataFlowController? This is an interface package, right? https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/CharArrayRecord.java File asterix-external-data/src/main/java/org/apache/asterix/external/input/record/CharArrayRecord.java: Line 29: import com.couchbase.client.deps.io.netty.buffer.ByteBuf; This is an ugly dependency, can we move the "set" method that uses it as a static method to its only consumer? https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/GenericRecord.java File asterix-external-data/src/main/java/org/apache/asterix/external/input/record/GenericRecord.java: Line 58: public Class<? extends T> getRecordClass() { It seems that we actually don't use this method. https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/CouchbaseReader.java File asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/CouchbaseReader.java: Line 141: if (event.partialUpdate()) { I assume that this is just some temporary emptyness, right? https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/HDFSRecordReader.java File asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/HDFSRecordReader.java: Line 98: record.setRecord(value); Oh, look, it was called "set"! https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/factory/AbstractStreamRecordReaderFactory.java File asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/factory/AbstractStreamRecordReaderFactory.java: Line 19: package org.apache.asterix.external.input.record.reader.factory; Hmm, maybe it would make more sense to organize the reader package by reader source than separating the readers from the factories? https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/factory/CouchbaseReaderFactory.java File asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/factory/CouchbaseReaderFactory.java: Line 82: throw new AsterixException("Unspesified bucket"); a/Unspesified/Unspecified/ ? Line 85: throw new AsterixException("Unspesified Couchbase nodes"); a/Unspesified/Unspecified/ ? https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedMetaComputeNodePushable.java File asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedMetaComputeNodePushable.java: Line 190: } catch (InterruptedException e) { Why would we ignore this? Somebody called Thread.interrupt(). https://asterix-gerrit.ics.uci.edu/#/c/585/9/asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/std/NoTupleSourceRuntimeFactory.java File asterix-runtime/src/main/java/org/apache/asterix/runtime/operators/std/NoTupleSourceRuntimeFactory.java: Line 45: }; Revert the file? -- To view, visit https://asterix-gerrit.ics.uci.edu/585 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id862ce9e9b1360864c6976f2aea2137092f51203 Gerrit-PatchSet: 9 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
