[GitHub] [hudi] leesf commented on a change in pull request #1842: [HUDI-1037]Introduce a write committed callback hook
leesf commented on a change in pull request #1842: URL: https://github.com/apache/hudi/pull/1842#discussion_r457312361 ## File path: hudi-client/src/main/java/org/apache/hudi/exception/HoodieCommitCallbackException.java ## @@ -0,0 +1,36 @@ +/* + * 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.hudi.exception; + +import org.apache.hudi.callback.HoodieWriteCommitCallback; + +/** + * Exception thrown for any higher level errors when {@link HoodieWriteCommitCallback} is executing a callback. + */ +public class HoodieCommitCallbackException extends HoodieException { + + public HoodieCommitCallbackException(String msg, Throwable e) { +super(msg, e); + } + + public HoodieCommitCallbackException(String msg) { +super(msg); + } + Review comment: extra line 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: us...@infra.apache.org
[GitHub] [hudi] leesf commented on a change in pull request #1842: [HUDI-1037]Introduce a write committed callback hook
leesf commented on a change in pull request #1842: URL: https://github.com/apache/hudi/pull/1842#discussion_r457311954 ## File path: hudi-client/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java ## @@ -632,6 +632,33 @@ public FileSystemViewStorageConfig getClientSpecifiedViewStorageConfig() { return clientSpecifiedViewStorageConfig; } + /** + * Commit call back configs. + */ + public boolean enableWriteCommitCallback() { +return Boolean.parseBoolean(props.getProperty(HoodieWriteCommitCallbackConfig.CALLBACK_ON)); + } + + public String getCallbackType() { +return props.getProperty(HoodieWriteCommitCallbackConfig.CALLBACK_TYPE_PROP); + } + + public String getCallbackClass() { +return props.getProperty(HoodieWriteCommitCallbackConfig.CALLBACK_CLASS_PROP); + } + + public String getCallbackRestUrl() { +return props.getProperty(HoodieWriteCommitCallbackConfig.CALLBACK_HTTP_URL_PROP); + } + + public int getCallbackRestTimeout() { +return Integer.parseInt(props.getProperty(HoodieWriteCommitCallbackConfig.CALLBACK_TIMEOUT_SECONDS)); + } + + public String getCallbackRestApiKey() { +return props.getProperty(HoodieWriteCommitCallbackConfig.CALLBACK_API_KEY); + } Review comment: should we move these methods into `HoodieHttpWriteCommitCallback.java`, I am worry about the explode of other callback methods in `HoodieWriteConfig` 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: us...@infra.apache.org
[GitHub] [hudi] leesf commented on a change in pull request #1842: [HUDI-1037]Introduce a write committed callback hook
leesf commented on a change in pull request #1842: URL: https://github.com/apache/hudi/pull/1842#discussion_r457309449 ## File path: hudi-client/src/main/java/org/apache/hudi/config/HoodieWriteCommitCallbackConfig.java ## @@ -0,0 +1,108 @@ +/* + * 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.hudi.config; + +import org.apache.hudi.callback.common.HoodieCommitCallbackType; +import org.apache.hudi.common.config.DefaultHoodieConfig; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.util.Properties; + +/** + * Write callback related config. + */ +public class HoodieWriteCommitCallbackConfig extends DefaultHoodieConfig { + + public static final String CALLBACK_ON = "hoodie.write.commit.callback.on"; + public static final boolean DEFAULT_CALLBACK_ON = false; + + public static final String CALLBACK_TYPE_PROP = "hoodie.write.commit.callback.type"; + public static final String DEFAULT_CALLBACK_TYPE_PROP = HoodieCommitCallbackType.HTTP.name(); + + public static final String CALLBACK_CLASS_PROP = "hoodie.write.commit.callback.class"; + public static final String DEFAULT_CALLBACK_CLASS_PROP = ""; + + // * REST callback configs * + public static final String CALLBACK_HTTP_URL_PROP = "hoodie.write.commit.callback.http.url"; + public static final String CALLBACK_API_KEY = "hoodie.write.commit.callback.http.api.key"; + public static final String DEFAULT_CALLBACK_API_KEY = "hudi_write_commit_callback"; + public static final String CALLBACK_TIMEOUT_SECONDS = "hoodie.write.commit.callback.rest.timeout.seconds"; + public static final String DEFAULT_CALLBACK_TIMEOUT_SECONDS = "3"; + + private HoodieWriteCommitCallbackConfig(Properties props) { +super(props); + } + + public static HoodieWriteCommitCallbackConfig.Builder newBuilder() { +return new HoodieWriteCommitCallbackConfig.Builder(); + } + + public static class Builder { + +private final Properties props = new Properties(); + +public HoodieWriteCommitCallbackConfig.Builder fromFile(File propertiesFile) throws IOException { + try (FileReader reader = new FileReader(propertiesFile)) { +this.props.load(reader); +return this; + } +} + +public HoodieWriteCommitCallbackConfig.Builder fromProperties(Properties props) { + this.props.putAll(props); + return this; +} + +public HoodieWriteCommitCallbackConfig.Builder writeCommitCallbackOn(String callbackOn) { + props.setProperty(CALLBACK_ON, String.valueOf(callbackOn)); Review comment: no need use `String.valueOf` here 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: us...@infra.apache.org
[GitHub] [hudi] leesf commented on a change in pull request #1842: [HUDI-1037]Introduce a write committed callback hook
leesf commented on a change in pull request #1842: URL: https://github.com/apache/hudi/pull/1842#discussion_r457309211 ## File path: hudi-client/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java ## @@ -632,6 +632,33 @@ public FileSystemViewStorageConfig getClientSpecifiedViewStorageConfig() { return clientSpecifiedViewStorageConfig; } + /** + * Commit call back configs. + */ + public boolean enableWriteCommitCallback() { Review comment: keep consistent with callback_on above, so `writeCommitCallbackOn`? 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: us...@infra.apache.org
[GitHub] [hudi] leesf commented on a change in pull request #1842: [HUDI-1037]Introduce a write committed callback hook
leesf commented on a change in pull request #1842: URL: https://github.com/apache/hudi/pull/1842#discussion_r456410986 ## File path: hudi-client/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java ## @@ -632,6 +632,33 @@ public FileSystemViewStorageConfig getClientSpecifiedViewStorageConfig() { return clientSpecifiedViewStorageConfig; } + /** + * Commit call back configs. + */ + public boolean isWriteCommitCallBackOn() { Review comment: enableWriteCommitCallback? 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: us...@infra.apache.org
[GitHub] [hudi] leesf commented on a change in pull request #1842: [HUDI-1037]Introduce a write committed callback hook
leesf commented on a change in pull request #1842: URL: https://github.com/apache/hudi/pull/1842#discussion_r456409913 ## File path: hudi-client/src/main/java/org/apache/hudi/callback/impl/HoodieHttpWriteCommitCallBack.java ## @@ -0,0 +1,64 @@ +/* + * 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.hudi.callback.impl; + +import org.apache.hudi.callback.HoodieWriteCommitCallBack; +import org.apache.hudi.callback.client.http.HoodieWriteCallBackHttpClient; +import org.apache.hudi.callback.common.HoodieBaseCommitCallBackMessage; +import org.apache.hudi.config.HoodieWriteConfig; +import org.apache.log4j.LogManager; +import org.apache.log4j.Logger; +import org.codehaus.jackson.map.ObjectMapper; + +import java.io.IOException; + +/** + * A http implementation of {@link HoodieWriteCommitCallBack}. + */ +public class HoodieHttpWriteCommitCallBack implements HoodieWriteCommitCallBack { + + private static final Logger LOG = LogManager.getLogger(HoodieHttpWriteCommitCallBack.class); + + private final HoodieWriteCallBackHttpClient client; + private final HoodieWriteConfig config; + + public HoodieHttpWriteCommitCallBack(HoodieWriteConfig config) { +this.config = config; +this.client = new HoodieWriteCallBackHttpClient(config.getCallBackRestApiKey(), +config.getCallBackRestUrl(), +config.getCallBackRestTimeout()); + } + + @Override + public void call(String commitTime) { +// create callback message +HoodieBaseCommitCallBackMessage callBackMessage = +new HoodieBaseCommitCallBackMessage(commitTime, config.getTableName()); +// convert to json +ObjectMapper mapper = new ObjectMapper(); +String callbackMsg = null; +try { + callbackMsg = mapper.writeValueAsString(callBackMessage); +} catch (IOException e) { + LOG.error("CallBack service convert message to json failed", e); Review comment: would be warn 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: us...@infra.apache.org
[GitHub] [hudi] leesf commented on a change in pull request #1842: [HUDI-1037]Introduce a write committed callback hook
leesf commented on a change in pull request #1842: URL: https://github.com/apache/hudi/pull/1842#discussion_r456409337 ## File path: hudi-client/src/main/java/org/apache/hudi/callback/HoodieWriteCommitCallBack.java ## @@ -0,0 +1,33 @@ +/* + * 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.hudi.callback; + +/** + * A callback interface help to call back when a write commit completes successfully. + */ +public interface HoodieWriteCommitCallBack { Review comment: HoodieWriteCommitCallback 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: us...@infra.apache.org