yihua commented on a change in pull request #4680:
URL: https://github.com/apache/hudi/pull/4680#discussion_r793319709



##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/async/AsyncClusteringService.java
##########
@@ -19,8 +19,8 @@
 
 package org.apache.hudi.async;
 
-import org.apache.hudi.client.AbstractClusteringClient;
-import org.apache.hudi.client.AbstractHoodieWriteClient;
+import org.apache.hudi.client.BaseClusterer;

Review comment:
       `BaseClusteringClient`?

##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/userdefined/AbstractUserDefinedMetricsReporter.java
##########
@@ -7,38 +7,31 @@
  * "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
+ *   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.
+ * 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.hudi.metrics.userdefined;
 
+import org.apache.hudi.metrics.custom.CustomizableMetricsReporter;
+
 import com.codahale.metrics.MetricRegistry;
-import org.apache.hudi.metrics.MetricsReporter;
+
 import java.util.Properties;
 
 /**
- * Abstract class of user defined metrics reporter.
+ * @deprecated Extend {@link CustomizableMetricsReporter} instead.
  */
-public abstract class AbstractUserDefinedMetricsReporter extends 
MetricsReporter {
-  private Properties props;
-  private MetricRegistry registry;
+@Deprecated
+public abstract class AbstractUserDefinedMetricsReporter extends 
CustomizableMetricsReporter {

Review comment:
       Great that the backward compatibility is considered :)

##########
File path: hudi-cli/src/main/java/org/apache/hudi/cli/commands/SparkMain.java
##########
@@ -458,7 +458,7 @@ protected static int 
upgradeOrDowngradeTable(JavaSparkContext jsc, String basePa
             
.setLoadActiveTimelineOnLoad(false).setConsistencyGuardConfig(config.getConsistencyGuardConfig())
             .setLayoutVersion(Option.of(new 
TimelineLayoutVersion(config.getTimelineLayoutVersion()))).build();
     try {
-      new UpgradeDowngrade(metaClient, config, new 
HoodieSparkEngineContext(jsc), SparkUpgradeDowngradeHelper.getInstance())
+      new Versioner(metaClient, config, new HoodieSparkEngineContext(jsc), 
SparkUpgradeDowngradeHelper.getInstance())

Review comment:
       Not a strong opinion.  I see you renamed upgrade- and downgrade-related 
classes. This isn't in the scope of the class renaming originally.  If we agree 
that the new names are better, 

##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/OneToZeroDowngrader.java
##########
@@ -35,13 +35,13 @@
 /**
  * Downgrade handle to assist in downgrading hoodie table from version 1 to 0.
  */
-public class OneToZeroDowngradeHandler implements DowngradeHandler {
+public class OneToZeroDowngrader implements Downgrader {

Review comment:
       I’m not in favor of `<verb>-er` naming pattern.  We can discuss how such 
classes can be renamed.




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to