mmiklavc commented on a change in pull request #1458: METRON-2177 Upgrade Profiler for HBase 2.0.2 URL: https://github.com/apache/metron/pull/1458#discussion_r308915461
########## File path: metron-analytics/metron-profiler-client/src/test/java/org/apache/metron/profiler/client/HBaseProfileWriter.java ########## @@ -0,0 +1,63 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.metron.profiler.client; + +import org.apache.hadoop.hbase.client.Durability; +import org.apache.metron.hbase.ColumnList; +import org.apache.metron.hbase.client.HBaseClient; +import org.apache.metron.profiler.ProfileMeasurement; +import org.apache.metron.profiler.hbase.ColumnBuilder; +import org.apache.metron.profiler.hbase.RowKeyBuilder; + +/** + * Writes ProfileMeasurement values that can be read during automated testing. + */ +public class HBaseProfileWriter implements ProfileWriter { + + private RowKeyBuilder rowKeyBuilder; + private ColumnBuilder columnBuilder; + private HBaseClient hbaseClient; + + /** + * @param rowKeyBuilder Builds the row key for a {@link ProfileMeasurement}. + * @param columnBuilder Builds the columns associated with a {@link ProfileMeasurement}. + * @param hbaseClient Writes the {@link ProfileMeasurement} values to HBase. + */ + public HBaseProfileWriter(RowKeyBuilder rowKeyBuilder, ColumnBuilder columnBuilder, HBaseClient hbaseClient) { Review comment: I like the tight, clean, self-describing API of this class. Nice job. This class and interface are isolated to writing only for tests. If this is net-new, might it be simpler to have a profiler DAO layer that does CRUD and can be used in other acceptance-level integration tests? It would then look something like: ``` interface ProfileDAO { createProfile readProfile updateProfile deleteProfile } class HBaseProfileDAO implements ProfileDAO {...} ``` And from a test perspective, it gets covered with 2 tiers of integration test (unit tests are not appropriate here because we don't want to unit test and mock code functionality that we don't own, as much as possible) 1. HBaseProfileDAOIntegrationTest - tests the basic exposed CRUD operations in isolation against HBase. Some devs actually duplicate all the CRUD code in their tests, but I've never seen much extra value in that. I would just use the DAO to write method to perform the setup and then validate the results of the other operations from the same DAO. 2. A more dynamic e2e acceptance test that verifies integration with a working DAO. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
