Till Westmann has posted comments on this change. Change subject: Add Asterix Extension Manager ......................................................................
Patch Set 13: (16 comments) Second round of comments. https://asterix-gerrit.ics.uci.edu/#/c/1017/13/asterixdb/asterix-app/src/test/java/org/apache/asterix/api/http/servlet/ConnectorAPIServletTest.java File asterixdb/asterix-app/src/test/java/org/apache/asterix/api/http/servlet/ConnectorAPIServletTest.java: Line 100: new JSONTokener(new InputStreamReader(new ByteArrayInputStream(outputStream.toByteArray()))); revert file? https://asterix-gerrit.ics.uci.edu/#/c/1017/13/asterixdb/asterix-common/src/main/resources/schema/asterix-conf.xsd File asterixdb/asterix-common/src/main/resources/schema/asterix-conf.xsd: Line 114: <xs:element name="extensions"> Indent? https://asterix-gerrit.ics.uci.edu/#/c/1017/13/asterixdb/asterix-common/src/main/resources/webui/errortemplate.html File asterixdb/asterix-common/src/main/resources/webui/errortemplate.html: Line 19: <div class="accordion" id="errorblock"> Would be really nice if this were not part of asterix-common ... https://asterix-gerrit.ics.uci.edu/#/c/1017/13/asterixdb/asterix-common/src/main/resources/webui/static/js/smoothie.js File asterixdb/asterix-common/src/main/resources/webui/static/js/smoothie.js: Line 37: * resetBoundsInterval: 3000 > ws all over this page But that's not our code ... https://asterix-gerrit.ics.uci.edu/#/c/1017/13/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/MetadataIndex.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/MetadataIndex.java: Line 89: throw new IllegalArgumentException("Unequal number of key types and names given."); Is this something that can ever happen in a working system? If not, should we just throw an AssertionError here? https://asterix-gerrit.ics.uci.edu/#/c/1017/13/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlDataSource.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlDataSource.java: Line 232: throws AlgebricksException; I'm not sure that this is the right abstraction. Clearly data seource and scan runtimes are closely related, but this seems a little too close. We might want to revisit this at a later point in time. https://asterix-gerrit.ics.uci.edu/#/c/1017/13/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/IMutationDataSource.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/IMutationDataSource.java: Line 26: public interface IMutationDataSource { What is a MutationDataSource? https://asterix-gerrit.ics.uci.edu/#/c/1017/13/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Dataset.java: Line 37: public static final byte UPSERT = 0x03; What are these constants? Can we explain them? https://asterix-gerrit.ics.uci.edu/#/c/1017/13/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/feeds/FeedMetadataUtil.java: Line 296: public static void increaseCardinality(JobSpecification spec, FeedRuntimeType compute, int requiredCardinality, Method seems to be unused - but maybe we'll need it later? Line 305: int requiredCardinality, List<String> currentLocations) throws AsterixException { Method seems to be unused - but maybe we'll need it later? Line 312: throws AsterixException { Method seems to be unused - but maybe we'll need it later? https://asterix-gerrit.ics.uci.edu/#/c/1017/13/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/base/AInt32.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/base/AInt32.java: Line 26: import org.json.JSONObject; Revert the file? https://asterix-gerrit.ics.uci.edu/#/c/1017/13/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/job/listener/MultiTransactionJobEventListenerFactory.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/job/listener/MultiTransactionJobEventListenerFactory.java: Line 34: public class MultiTransactionJobEventListenerFactory implements IJobletEventListenerFactory { Some comment what this class does? https://asterix-gerrit.ics.uci.edu/#/c/1017/13/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/dataflow/ActivityId.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/dataflow/ActivityId.java: Line 52: public void setOperatorDescriptorId(OperatorDescriptorId odId) { Do we really need to reset this? Or can we ensure that this is only set once? https://asterix-gerrit.ics.uci.edu/#/c/1017/13/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/dataflow/IConnectorDescriptor.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/dataflow/IConnectorDescriptor.java: Line 147: public ConnectorDescriptorId clone(IConnectorDescriptorRegistry registry); Why would the clone of an IConnectorDescriptor be a ConnectorDescriptorId? https://asterix-gerrit.ics.uci.edu/#/c/1017/13/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/dataflow/IOperatorDescriptor.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/dataflow/IOperatorDescriptor.java: Line 47: void setOperatorId(OperatorDescriptorId id); Could we avoid resetting ids? When do we need that? -- To view, visit https://asterix-gerrit.ics.uci.edu/1017 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I280268495cc3aad00f898cba21f7299f7120ce5c Gerrit-PatchSet: 13 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Steven Jacobs <sjaco...@ucr.edu> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes
