You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "ryanjclark (via GitHub)" <gi...@apache.org> on 2024/10/11 17:25:24 UTC
[PR] add checkClassName regex func [iceberg]
ryanjclark opened a new pull request, #11303:
URL: https://github.com/apache/iceberg/pull/11303
We have a custom wrapper on the ConnectDistributed class which does not match the regex check here. This PR expands the regex to be more generous but is still reasonable, especially considering the properties check afterwards.
Closes an issue in [tabular.io/iceberg-kafka-connect](https://github.com/tabular-io/iceberg-kafka-connect/issues/301)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add checkClassName regex func [iceberg]
Posted by "bryanck (via GitHub)" <gi...@apache.org>.
bryanck commented on PR #11303:
URL: https://github.com/apache/iceberg/pull/11303#issuecomment-2417508693
Thanks for the PR @ryanjclark !
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add checkClassName regex func [iceberg]
Posted by "bryanck (via GitHub)" <gi...@apache.org>.
bryanck commented on code in PR #11303:
URL: https://github.com/apache/iceberg/pull/11303#discussion_r1803734111
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/IcebergSinkConfig.java:
##########
@@ -406,6 +406,10 @@ public JsonConverter jsonConverter() {
return jsonConverter;
}
+ static boolean checkClassName(String className) {
Review Comment:
One more nit, could you add `@VisibleForTesting`? That makes it clear why it isn't private.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add checkClassName regex func [iceberg]
Posted by "bryanck (via GitHub)" <gi...@apache.org>.
bryanck commented on code in PR #11303:
URL: https://github.com/apache/iceberg/pull/11303#discussion_r1803558321
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/IcebergSinkConfig.java:
##########
@@ -406,6 +406,10 @@ public JsonConverter jsonConverter() {
return jsonConverter;
}
+ public static boolean checkClassName(String className) {
Review Comment:
One nit, can you make this `private`?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add checkClassName regex func [iceberg]
Posted by "bryanck (via GitHub)" <gi...@apache.org>.
bryanck merged PR #11303:
URL: https://github.com/apache/iceberg/pull/11303
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org