abdullah alamoudi has posted comments on this change.

Change subject: Feed Fixes and Cleanup
......................................................................


Patch Set 5:

(72 comments)

https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/ExternalLibraryBootstrap.java
File 
asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/ExternalLibraryBootstrap.java:

Line 40: import org.apache.asterix.external.api.IDataSourceAdapter.AdapterType;
> remove unused import.
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/FeedBootstrap.java
File 
asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/FeedBootstrap.java:

Line 33:                     + builder.append("create dataverse " + 
FeedConstants.FEEDS_METADATA_DV + ";" + "\n"));
> why are these dataverse created?
Done


Line 52:             builder.append(
> why are these dataverse created?
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/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 287:         // get node stores
> Remove comment and format the class.
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-common/src/main/java/org/apache/asterix/common/api/INodeFeedManager.java
File 
asterix-common/src/main/java/org/apache/asterix/common/api/INodeFeedManager.java:

Line 29:     public String getNodeId();
> This is not needed in this interface.
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/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 2:  * Licensed to the Apache Software Foundation (ASF) under one
> Reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-common/src/main/java/org/apache/asterix/common/parse/ITupleForwarder.java
File 
asterix-common/src/main/java/org/apache/asterix/common/parse/ITupleForwarder.java:

Line 43:     public void addTuple(ArrayTupleBuilder tb) throws 
HyracksDataException, InterruptedException;
> I don't think InterruptedException is needed.
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/adapter/factory/GenericAdapterFactory.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/adapter/factory/GenericAdapterFactory.java:

Line 2:  * Licensed to the Apache Software Foundation (ASF) under one
> Reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/api/IDataFlowController.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/api/IDataFlowController.java:

Line 41:      * 5. start(writer)
> add new events
Done


Line 46:     public boolean stop() throws Exception;
> change exceptions to HyracksDataException
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedRecordDataFlowController.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedRecordDataFlowController.java:

Line 34:     protected MutableBoolean closed = new MutableBoolean(false);
> change to atomicboolean
Done


Line 56:         }
> move closing to finally.
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedStreamDataFlowController.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedStreamDataFlowController.java:

Line 44:             tupleForwarder.close();
> move close to finally
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedTupleForwarder.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedTupleForwarder.java:

Line 46:         this.appender = new FrameTupleAppender();
> pass frame in constructor
Done


Line 67:                 throw new IllegalStateException();
> add message here.
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/dataset/adapter/AdapterIdentifier.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/dataset/adapter/AdapterIdentifier.java:

Line 6:  * to you under the Apache License, Version 2.0 (the
> reformat
Done


Line 28:     private final String namespace; // what is the adapter namespace?
> dataverse
Done


Line 29:     private final String name;      // what is the adapter name?
> adapterName
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/dataset/adapter/GenericAdapter.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/dataset/adapter/GenericAdapter.java:

Line 57:         return controller.pause();
> resume
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IAdapterExecutor.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IAdapterExecutor.java:

Line 23: public interface IAdapterExecutor {
> Unused interface. Remove
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IFeedOperatorOutputSideHandler.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IFeedOperatorOutputSideHandler.java:

Line 27:  * and we can probably remove it completely.
> I should +2 the change just to get this in :)
I know ;-)


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IPrimaryFeed.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/api/IPrimaryFeed.java:

Line 24: 
> I don't think we need the interface, just add these methods to IFeed
Done


Line 25:     public Map<String, String> getAdaptorConfiguration();
> change to Adapter
Done


Line 27:     public String getAdaptorName();
> change to Adapter
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/CollectTransformFeedFrameWriter.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/CollectTransformFeedFrameWriter.java:

Line 2:  * Licensed to the Apache Software Foundation (ASF) under one
> reformat
Done


Line 53:         tupleAppender = new FrameTupleAppender();
> just pass the frame to the constructor
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/DataBucket.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/DataBucket.java:

Line 6:  * to you under the Apache License, Version 2.0 (the
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedExceptionHandler.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedExceptionHandler.java:

Line 39:     private final FrameTupleAccessor fta;
> Add TODO to re-enable logging
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedFrameCollector.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedFrameCollector.java:

Line 4:  * distributed with this work for additional information
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedRuntimeInputHandler.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedRuntimeInputHandler.java:

Line 8:  * with the License. You may obtain a copy of the License at
> reformat
Done


Line 86:         this.connectionId = connectionId;
> call one constructor from the other
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedActivityIdFactory.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedActivityIdFactory.java:

Line 6:  * to you under the Apache License, Version 2.0 (the
> reformat
Done


Line 21: public class FeedActivityIdFactory {
> Remove this class
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/FeedRuntimeReport.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/FeedRuntimeReport.java:

Line 21: public class FeedRuntimeReport {
> Remove!
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/IngestionRuntime.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/runtime/IngestionRuntime.java:

Line 5:  * regarding copyright ownership. The ASF licenses this file
> Reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/FileSystemFeedProgressTracker.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/watch/FileSystemFeedProgressTracker.java:

Line 19: public class FileSystemFeedProgressTracker {
> Remove!
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/TwitterPushRecordReader.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/input/record/reader/TwitterPushRecordReader.java:

Line 41:     private boolean closed;
> = false;
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/AInputStreamReader.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/AInputStreamReader.java:

Line 36:     @Override
> Remove, let the call go to super
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/LocalFSInputStreamProvider.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/LocalFSInputStreamProvider.java:

Line 3:  * or more contributor license agreements. See the NOTICE file
> Reformat
Done


Line 45:                     
feedLogFileSplits[partition].getLocalFile().getFile().getPath(), 0, 
ctx.getIOManager()).getFile();
> You need to pass the IO device of the partition from the FileSplit
Done


Line 58:         } catch (Exception e) {
> You don't need to wrap the exception here
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/factory/LocalFSInputStreamProviderFactory.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/input/stream/factory/LocalFSInputStreamProviderFactory.java:

Line 7:  * "License"); you may not use this file except in compliance
> Reformat
Done


Line 99:                 if (!trimmedValue.contains("://")) {
> If this condition is being checking in multiple locations, please move it t
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunction.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/library/ExternalFunction.java:

Line 4:  * distributed with this work for additional information
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/library/ResultCollector.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/library/ResultCollector.java:

Line 8:  * with the License. You may obtain a copy of the License at
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/library/java/JObjectUtil.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/library/java/JObjectUtil.java:

Line 5:  * regarding copyright ownership. The ASF licenses this file
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/operators/ExternalDatasetIndexesRecoverOperatorDescriptor.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/operators/ExternalDatasetIndexesRecoverOperatorDescriptor.java:

Line 4:  * distributed with this work for additional information
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedCollectOperatorNodePushable.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedCollectOperatorNodePushable.java:

Line 5:  * regarding copyright ownership. The ASF licenses this file
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedMessageOperatorNodePushable.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/operators/FeedMessageOperatorNodePushable.java:

Line 6:  * to you under the Apache License, Version 2.0 (the
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/DataflowUtils.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/DataflowUtils.java:

Line 50:         if (ExternalDataUtils.isFeed(configuration)) {
> Add a TODO to pass this value in the configuration and avoid this check for
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataCompatibilityUtils.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataCompatibilityUtils.java:

Line 105:             if 
(adapterClassname.equalsIgnoreCase(ExternalDataConstants.ALIAS_FILE_FEED_ADAPTER))
 {
> change equals
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataConstants.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataConstants.java:

Line 5:  * regarding copyright ownership. The ASF licenses this file
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataUtils.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataUtils.java:

Line 5:  * regarding copyright ownership. The ASF licenses this file
> reformat
Done


Line 238:             throws AsterixException {
> Remove exception
Done


Line 246:     public static boolean waitForData(Map<String, String> 
configuration) {
> rename this method
Done


Line 248:             return true;
> return false;
true is the default. keeping this as it is.


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedConstants.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedConstants.java:

Line 9:  * http://www.apache.org/licenses/LICENSE-2.0
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedFrameUtil.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedFrameUtil.java:

Line 34:     public static ByteBuffer getSlicedFrame(IHyracksTaskContext ctx, 
int tupleIndex, FrameTupleAccessor fta)
> This was replaced by the method below. Please remove it.
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedLogManager.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedLogManager.java:

Line 5:  * regarding copyright ownership. The ASF licenses this file
> reformat
Done


Line 112:         new File(dir.toAbsolutePath().toString() + File.separator + 
PROGRESS_LOG_FILE_NAME).createNewFile();
> use the constructor with two arguments.
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedUtils.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedUtils.java:

Line 6:  * to you under the Apache License, Version 2.0 (the
> reformat
Done


Line 37:     public static String getFeedPointKeyRep(IFeed feed, List<String> 
appliedFunctions) {
> Unused method
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java:

Line 7:  * "License"); you may not use this file except in compliance
> reformat
Done


Line 101:         if (it == null)
> {
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java:

Line 33:     public static void traverse(final LinkedList<File> files, File 
root, final String expression,
> See if you can replace this method by FileUtils.iterateFilesAndDirs(.)
Added a TODO :)


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-external-data/src/test/java/org/apache/asterix/external/library/UpperCaseFunction.java
File 
asterix-external-data/src/test/java/org/apache/asterix/external/library/UpperCaseFunction.java:

Line 8:  * with the License. You may obtain a copy of the License at
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-metadata/src/main/java/org/apache/asterix/metadata/api/IMetadataEntity.java
File 
asterix-metadata/src/main/java/org/apache/asterix/metadata/api/IMetadataEntity.java:

Line 26: public interface IMetadataEntity<E> extends Serializable {
> Please change it to T
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/MetadataBootstrap.java
File 
asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/MetadataBootstrap.java:

Line 3:  * or more contributor license agreements. See the NOTICE file
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java
File 
asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java:

Line 5:  * regarding copyright ownership. The ASF licenses this file
> reformat
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/MetadataFeedUtil.java
File 
asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/MetadataFeedUtil.java:

Line 6:  * to you under the Apache License, Version 2.0 (the
> reformat
Done


Line 91: public class MetadataFeedUtil {
> FeedMetadataUtil
Done


https://asterix-gerrit.ics.uci.edu/#/c/574/5/asterix-om/src/main/java/org/apache/asterix/om/util/AsterixClusterProperties.java
File 
asterix-om/src/main/java/org/apache/asterix/om/util/AsterixClusterProperties.java:

Line 3:  * or more contributor license agreements. See the NOTICE file
> Checkout this file from master
Done


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/574
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4e8db26a810efd1fbaa52ceeb3efd0c8328ab070
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to