You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2022/05/28 09:25:50 UTC
[GitHub] [hudi] Clcanny opened a new pull request, #5709: [MINOR] Print error msg when delta streamer config error happens
Clcanny opened a new pull request, #5709:
URL: https://github.com/apache/hudi/pull/5709
## *Tips*
- *Thank you very much for contributing to Apache Hudi.*
- *Please review https://hudi.apache.org/contribute/how-to-contribute before opening a pull request.*
## What is the purpose of the pull request
Print more error infos when new source failed with config error.
For example, when we new JsonKafkaSource.java with error checkpoint config.
1. `UtilHelpers` tries `JsonKafkaSource(TypedProperties properties, JavaSparkContext sparkContext, SparkSession sparkSession, SchemaProvider schemaProvider, HoodieDeltaStreamerMetrics metrics` first, and it throws `HoodieException`(That `HoodieException` contains useful error msg).
2. Then `UtilHelpers` tries `JsonKafkaSource(TypedProperties properties, JavaSparkContext sparkContext, SparkSession sparkSession, SchemaProvider schemaProvider`, and it throws `HoodieException`.
3. User only knows a not related error msg, and wonder why program complains not such method.
## Brief change log
1. Print error msg if not `NoSuchMethodException` in `UtilHelpers`.
## Verify this pull request
This pull request is a trivial rework / code cleanup without any test coverage.
## Committer checklist
- [ ] Has a corresponding JIRA in PR title & commit
- [Y] Commit message is descriptive of the change
- [ ] CI is green
- [ ] Necessary doc changes done or have another open PR
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #5709:
URL: https://github.com/apache/hudi/pull/5709#issuecomment-1140227012
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2b22c18e0d46bc747c94fe5c95d7e3c824298451",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8989",
"triggerID" : "2b22c18e0d46bc747c94fe5c95d7e3c824298451",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2b22c18e0d46bc747c94fe5c95d7e3c824298451 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8989)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] Clcanny commented on a diff in pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
Clcanny commented on code in PR #5709:
URL: https://github.com/apache/hudi/pull/5709#discussion_r884144239
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/UtilHelpers.java:
##########
@@ -122,6 +122,9 @@ public static Source createSource(String sourceClass, TypedProperties cfg, JavaS
HoodieDeltaStreamerMetrics.class},
cfg, jssc, sparkSession, schemaProvider, metrics);
} catch (HoodieException e) {
+ if (e.getCause() != null && !(e.getCause() instanceof NoSuchMethodException)) {
+ LOG.error("Could not load source class " + sourceClass, e);
Review Comment:
Thanks for your review! I think we shouldn't print error msg of NoSuchMethodException, because it is noisy and user doesn't need to know that.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] Clcanny commented on a diff in pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
Clcanny commented on code in PR #5709:
URL: https://github.com/apache/hudi/pull/5709#discussion_r884144348
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/UtilHelpers.java:
##########
@@ -122,6 +122,9 @@ public static Source createSource(String sourceClass, TypedProperties cfg, JavaS
HoodieDeltaStreamerMetrics.class},
cfg, jssc, sparkSession, schemaProvider, metrics);
} catch (HoodieException e) {
+ if (e.getCause() != null && !(e.getCause() instanceof NoSuchMethodException)) {
+ LOG.error("Could not load source class " + sourceClass, e);
Review Comment:
For a Source class which just implements constructor of type: `(TypedProperties properties, JavaSparkContext sparkContext, SparkSession sparkSession, SchemaProvider schemaProvider)`, doesn't implement constructor of type: `(TypedProperties properties, JavaSparkContext sparkContext, SparkSession sparkSession, SchemaProvider schemaProvider, HoodieDeltaStreamerMetrics metrics)`.
Then if we don't judge like that, there are a lot of error messages.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #5709:
URL: https://github.com/apache/hudi/pull/5709#issuecomment-1140241432
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2b22c18e0d46bc747c94fe5c95d7e3c824298451",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8989",
"triggerID" : "2b22c18e0d46bc747c94fe5c95d7e3c824298451",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2b22c18e0d46bc747c94fe5c95d7e3c824298451 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=8989)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan closed pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
xushiyan closed pull request #5709: [MINOR] Print error msg when delta streamer config error happens
URL: https://github.com/apache/hudi/pull/5709
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #5709:
URL: https://github.com/apache/hudi/pull/5709#issuecomment-1140226338
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2b22c18e0d46bc747c94fe5c95d7e3c824298451",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "2b22c18e0d46bc747c94fe5c95d7e3c824298451",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2b22c18e0d46bc747c94fe5c95d7e3c824298451 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a diff in pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
yanghua commented on code in PR #5709:
URL: https://github.com/apache/hudi/pull/5709#discussion_r884142438
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/UtilHelpers.java:
##########
@@ -122,6 +122,9 @@ public static Source createSource(String sourceClass, TypedProperties cfg, JavaS
HoodieDeltaStreamerMetrics.class},
cfg, jssc, sparkSession, schemaProvider, metrics);
} catch (HoodieException e) {
+ if (e.getCause() != null && !(e.getCause() instanceof NoSuchMethodException)) {
+ LOG.error("Could not load source class " + sourceClass, e);
Review Comment:
I would suggest we print this error message directly. No need to add this condition `e.getCause() != null && !(e.getCause() instanceof NoSuchMethodException)`. WDYT?
You just want to log more information about this exception, right? Just log it.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] Clcanny commented on a diff in pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
Clcanny commented on code in PR #5709:
URL: https://github.com/apache/hudi/pull/5709#discussion_r884144348
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/UtilHelpers.java:
##########
@@ -122,6 +122,9 @@ public static Source createSource(String sourceClass, TypedProperties cfg, JavaS
HoodieDeltaStreamerMetrics.class},
cfg, jssc, sparkSession, schemaProvider, metrics);
} catch (HoodieException e) {
+ if (e.getCause() != null && !(e.getCause() instanceof NoSuchMethodException)) {
+ LOG.error("Could not load source class " + sourceClass, e);
Review Comment:
Maybe some Source class just implement constructor of `(TypedProperties properties, JavaSparkContext sparkContext, SparkSession sparkSession, SchemaProvider schemaProvider)`, doesn't implement `(TypedProperties properties, JavaSparkContext sparkContext, SparkSession sparkSession, SchemaProvider schemaProvider, HoodieDeltaStreamerMetrics metrics)`.
Then if we don't judge, there are a lot of error messages.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
xushiyan commented on PR #5709:
URL: https://github.com/apache/hudi/pull/5709#issuecomment-1140306902
As suggested in the comment replied above.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] Clcanny commented on a diff in pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
Clcanny commented on code in PR #5709:
URL: https://github.com/apache/hudi/pull/5709#discussion_r884144348
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/UtilHelpers.java:
##########
@@ -122,6 +122,9 @@ public static Source createSource(String sourceClass, TypedProperties cfg, JavaS
HoodieDeltaStreamerMetrics.class},
cfg, jssc, sparkSession, schemaProvider, metrics);
} catch (HoodieException e) {
+ if (e.getCause() != null && !(e.getCause() instanceof NoSuchMethodException)) {
+ LOG.error("Could not load source class " + sourceClass, e);
Review Comment:
Maybe some Source class just implement constructor of type: `(TypedProperties properties, JavaSparkContext sparkContext, SparkSession sparkSession, SchemaProvider schemaProvider)`, doesn't implement constructor of type: `(TypedProperties properties, JavaSparkContext sparkContext, SparkSession sparkSession, SchemaProvider schemaProvider, HoodieDeltaStreamerMetrics metrics)`.
Then if we don't judge, there are a lot of error messages.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] Clcanny commented on a diff in pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
Clcanny commented on code in PR #5709:
URL: https://github.com/apache/hudi/pull/5709#discussion_r884144348
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/UtilHelpers.java:
##########
@@ -122,6 +122,9 @@ public static Source createSource(String sourceClass, TypedProperties cfg, JavaS
HoodieDeltaStreamerMetrics.class},
cfg, jssc, sparkSession, schemaProvider, metrics);
} catch (HoodieException e) {
+ if (e.getCause() != null && !(e.getCause() instanceof NoSuchMethodException)) {
+ LOG.error("Could not load source class " + sourceClass, e);
Review Comment:
Maybe some Source class just implement constructor of `(TypedProperties properties, JavaSparkContext sparkContext, SparkSession sparkSession, SchemaProvider schemaProvider)`, and if we don't judge, there are a lot of error messages.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] xushiyan commented on a diff in pull request #5709: [MINOR] Print error msg when delta streamer config error happens
Posted by GitBox <gi...@apache.org>.
xushiyan commented on code in PR #5709:
URL: https://github.com/apache/hudi/pull/5709#discussion_r884162763
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/UtilHelpers.java:
##########
@@ -122,6 +122,9 @@ public static Source createSource(String sourceClass, TypedProperties cfg, JavaS
HoodieDeltaStreamerMetrics.class},
cfg, jssc, sparkSession, schemaProvider, metrics);
} catch (HoodieException e) {
+ if (e.getCause() != null && !(e.getCause() instanceof NoSuchMethodException)) {
+ LOG.error("Could not load source class " + sourceClass, e);
Review Comment:
@Clcanny this sort of logic is very unconventional. when there is an error, we log it. that's simple and straightforward. if you find a lot of unwanted logs, then put a fix on the root cause, instead of putting a band-aid fix like this.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org