You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/11/24 01:23:42 UTC
[GitHub] [beam] omarismail94 opened a new pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
omarismail94 opened a new pull request #13410:
URL: https://github.com/apache/beam/pull/13410
**Please** add a meaningful description for your change here
Replace Charset.defaultCharset() with StandardCharsets.UTF_8 so that reliance of encoding set in `locale` is not used.
The [defaultCharset()](https://docs.oracle.com/javase/7/docs/api/java/nio/charset/Charset.html#defaultCharset()) method relies on underlying OS system default.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam
.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website | Whitespace | Typescript
--- | --- | --- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | --- | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r552108921
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -69,6 +69,9 @@
private int carryOverBytes;
private byte[] carryOverByteArray;
+ @SuppressWarnings({
+ "unchecked" // [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Review comment:
Yes! Changed to `Suppress ForbidDefaultCharset checkstyle`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] iemejia commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
iemejia commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r551198450
##########
File path: sdks/java/build-tools/src/main/resources/beam/checkstyle.xml
##########
@@ -119,6 +119,14 @@ page at http://checkstyle.sourceforge.net/config.html -->
<property name="message" value="You are using raw guava, please use vendored guava classes."/>
</module>
+ <!-- Forbid use of Charset.defaultCharset()-->
+ <module name="RegexpSinglelineJava">
+ <property name="id" value="ForbidCharset"/>
Review comment:
nit: ultra minor (you can ignore me but good if changed) s/ForbidCharset/ForbidDefaultCharset
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r553698068
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
Oooh, OK! And because the constructor is private, it can only be accessed from `create()` method inside the class, meaning this is the only file I have to change 🤯
Ill give this a shot!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r553682842
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
> How about this - we can make this class (the private constructor and the create() method below) parameterized by Charset.
The variables that are passed in do not use any of the `Charset` methods though right?
>The places where its used as the default System.out/System.err in the Dataflow worker can pass in Charset.defaultCharset(), but the test can pass in Charsets.UTF_8
The test file has `StandardCharsets.UTF_8` replacing `Charset.defaultCharset()`.
Let me know if I am missing something @TheNeuralBit !
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
> How about this - we can make this class (the private constructor and the create() method below) parameterized by Charset.
The variables that are passed in do not use any of the `Charset` methods though right?
>The places where its used as the default System.out/System.err in the Dataflow worker can pass in Charset.defaultCharset(), but the test can pass in Charsets.UTF_8
The test file has `StandardCharsets.UTF_8` replacing `Charset.defaultCharset()`.
Let me know if I am missing something @TheNeuralBit !
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] iemejia commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
iemejia commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-750880808
I wonder if we could add a rule to avoid this use in the future maybe via [checkstyle](https://github.com/apache/beam/blob/master/sdks/java/build-tools/src/main/resources/beam/checkstyle.xml), otherwise we will be fixing this continuously.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-752758820
Thanks Brian, I'll give it a shot!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-753613247
retest this please
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r530008109
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
OK thanks, I thought that may be the case
How about this - we can make this class (the private constructor and the create() method below) parameterized by Charset. The places where its used as the default System.out/System.err in the Dataflow worker can pass in Charset.defaultCharset(), but the test can pass in Charsets.UTF_8
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r552375009
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -69,6 +69,9 @@
private int carryOverBytes;
private byte[] carryOverByteArray;
+ @SuppressWarnings({
+ "unchecked" // [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Review comment:
Aaaah, I finally found out what causes that. `checkstyleMain.enabled` is set to `false` in the [build.gradle](https://github.com/apache/beam/blob/master/runners/google-cloud-dataflow-java/worker/build.gradle#L118) for the worker project. I set it to `true`, then removed the SuppressionWarning, and when I run `checkstyleMain`, I can see the rule gets triggered. I also didn't know you can set the ID used as the label in, `SuppressionWarnings`, so I fixed that as well!
If you want to test, I think if you set the flag to `true` in the build.gradle file, and remove the SuppressionWarning, then run the check, it _should_ work
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-754730494
Run Java PreCommit
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r553701260
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
Will do, thanks!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r553699566
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
Well you'll also want to add an argument to `create()`, and update the places that call it, for example in the test: https://github.com/apache/beam/blob/df74d74ef295aa972b0d1ddef2429f427b988a25/runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactoryTest.java#L168
Would become `return JulHandlerPrintStreamAdapterFactory.create(handler, LOGGER_NAME, Level.INFO, StandardCharsets.UTF_8);`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] iemejia merged pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
iemejia merged pull request #13410:
URL: https://github.com/apache/beam/pull/13410
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r554016730
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
@TheNeuralBit I had to create a new PR for this as this got merged:
New PR: https://github.com/apache/beam/pull/13701
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r552169341
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
This hasn't been done yet has 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] iemejia commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
iemejia commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-754659544
Run Java PreCommit
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] iemejia commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
iemejia commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-750931216
Yes it seems a bit radical but this PR proves that the issue is around and I remember to have fixed this too in the past.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r552101064
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -69,6 +69,9 @@
private int carryOverBytes;
private byte[] carryOverByteArray;
+ @SuppressWarnings({
+ "unchecked" // [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Review comment:
Shouldn't this be ForbidDefaultCharset?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r536200396
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
@scwhittle does that work for you? I can make the change if so!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] iemejia merged pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
iemejia merged pull request #13410:
URL: https://github.com/apache/beam/pull/13410
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-752698729
> I don't know how checkstyle works, but I can try to figure it out!
If you have the bandwidth it would be great to do it as part of this PR! It could be left as a follow-up if you don't though.
We'd probably want to add another entry like this one, but with a regex for `Charset.defaultCharset()`:
https://github.com/apache/beam/blob/245cf2b8ccdca6b66269b4e473c1a8903631cb3f/sdks/java/build-tools/src/main/resources/beam/checkstyle.xml#L114-L120
You can test it out locally by running `./gradlew checkstyleMain checkstyleTest`
Since there are some rare cases where we need the default charset we'll need to be able to suppress the check. Based on https://stackoverflow.com/questions/27688426/ignoring-of-checkstyle-warnings-with-annotation-suppresswarnings it looks like this can be done with `@SuppressWarnings`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r540477688
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
Let's go ahead and do 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] nielsbasjes commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-732811394
A few weeks ago a colleague of mine mentioned he ran into a character encoding issue when building a Beam application.
I just spoke him and he pointed me to this comment that simply documents what he ran into: https://github.com/apache/beam/blob/master/sdks/java/io/xml/src/main/java/org/apache/beam/sdk/io/xml/XmlSource.java#L335
```
// We use Woodstox because the StAX implementation provided by OpenJDK reports
// character locations incorrectly. Note that Woodstox still currently reports *byte*
// locations incorrectly when parsing documents that contain multi-byte characters.
```
Apparently even with this fix in place the project as a whole still has character encoding problems.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-754754283
Third time the charm!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r541085981
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
OK, I will make the changes and re-commit. Thanks!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-753640732
retest this please
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-733093034
> Apparently even with this fix in place the project as a whole still has character encoding problems.
That issue is very much constrained to XmlIO, let's not throw out the whole project over it ;)
That comment dates back almost to the original contribution of Beam as the Dataflow SDK! We're using woodstox 4.4.1 which came out in Sep 2014. Maybe this is a simple as upgrading woodstox? It looks like BEAM-10883 is tracking that issue, I'll comment over there.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 edited a comment on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 edited a comment on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-732527680
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] scwhittle commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
scwhittle commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r529905037
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
This class is going to be used to replace the default System.out/System.err which appears to be specified to use the default charset.
So it seems better to keep it consistent IMO to avoid surprises when someone expects that charset writing to System.out, not UTF-8. If the test is incorrect, it should be fixed (by examining or setting the defaultCharset)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r554016730
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
@TheNeuralBit I had to create a new PR for this as this got merged:
New PR: https://github.com/apache/beam/pull/13701
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r551482024
##########
File path: sdks/java/build-tools/src/main/resources/beam/checkstyle.xml
##########
@@ -119,6 +119,14 @@ page at http://checkstyle.sourceforge.net/config.html -->
<property name="message" value="You are using raw guava, please use vendored guava classes."/>
</module>
+ <!-- Forbid use of Charset.defaultCharset()-->
+ <module name="RegexpSinglelineJava">
+ <property name="id" value="ForbidCharset"/>
Review comment:
Updated, thanks!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r552375009
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -69,6 +69,9 @@
private int carryOverBytes;
private byte[] carryOverByteArray;
+ @SuppressWarnings({
+ "unchecked" // [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Review comment:
Aaaah, I finally found out what causes that. `checkstyleMain.enabled` is set to `false` in the [build.gradle](https://github.com/apache/beam/blob/master/runners/google-cloud-dataflow-java/worker/build.gradle#L118) for the worker project. I set it to `true`, then removed the SuppressionWarning, and when I run `checkstyleMain`, I can see the rule gets triggered. I also didn't know you can set the ID used as the label in, `SuppressionWarning`, so I fixed that as well!
If you want to test, I think if you set the flag to `true` in the build.gradle file, and remove the SuppressionWarning, then run the check, it _should_ work
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-754867852
Run Python_PVR_Flink PreCommit
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-732527680
R: @TheNeuralBit @nielsbasjes
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] scwhittle commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
scwhittle commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r540782777
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
Sorry lost track of that comment, I believe it's only uses are for System.out/System.err and the test. But allowing the test to specify a specific charset so it passes reliably sounds like a good solution.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r553692685
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
What I was suggesting is that we should add a new argument to the private constructor and the static create() method: `Charset charset`. Then in the constructor we'll use `charset` instead of `Charset.defaultCharset()` to create `encoder`.
Currently the test is using `StandardCharsets.UTF_8`, but only to create the expected outputs. It also needs to create a `JulHandlerPrintStreamAdapterFactory` that uses `UTF_8` explicitly, so we need a way to pass that through. I think as it is this test could still fail if the default charset isn't UTF-8
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-754730740
https://ci-beam.apache.org/job/beam_PreCommit_Java_Phrase/2953/ flake looks like BEAM-8101
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r552169007
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -69,6 +69,9 @@
private int carryOverBytes;
private byte[] carryOverByteArray;
+ @SuppressWarnings({
+ "unchecked" // [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Review comment:
It's concerning that the precommit didn't break when this was suppressing the wrong warning. Have you seen the new checkstyle rule break checkstyleMain or checkstyleTest?
I tried to get it to fail locally, but even with the SuppressWarnings removed its not triggering on Charset.defaultCharset
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-750911509
@iemejia We do have a rule that stops you from using APIs that use the default charset implicitly (e.g. `String.getBytes()`), but it doesn't have a problem with the explicit `String.getBytes(Charset.defaultCharset())`. Maybe we could have a rule that blocks even Charset.defaultCharset() and require it to be suppressed
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on pull request #13410:
URL: https://github.com/apache/beam/pull/13410#issuecomment-752689288
> I wonder if we could add a rule to avoid this use in the future maybe via [checkstyle](https://github.com/apache/beam/blob/master/sdks/java/build-tools/src/main/resources/beam/checkstyle.xml), otherwise we will be fixing this continuously.
I don't know how checkstyle works, but I can try to figure it out!
Worst case is just brute-forcing current occurrences of `Charset.defaultCharset()`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r529153011
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -86,7 +86,7 @@ public void write(int i) throws IOException {
this.logger = Logger.getLogger(loggerName);
this.buffer = new StringBuilder();
this.decoder =
- Charset.defaultCharset()
+ StandardCharsets.UTF_8
Review comment:
@scwhittle I was wondering if you could comment on this since you added this line in https://github.com/apache/beam/pull/11096. Do you know if this change is safe? Is there a reason this needs to use `defaultCharset`?
As it is, `testLogRawBytes` below can fail if the system encoding isn't UTF-8
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [beam] omarismail94 commented on a change in pull request #13410: [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Posted by GitBox <gi...@apache.org>.
omarismail94 commented on a change in pull request #13410:
URL: https://github.com/apache/beam/pull/13410#discussion_r552375009
##########
File path: runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/JulHandlerPrintStreamAdapterFactory.java
##########
@@ -69,6 +69,9 @@
private int carryOverBytes;
private byte[] carryOverByteArray;
+ @SuppressWarnings({
+ "unchecked" // [BEAM-11327] Replace Charset.defaultCharset() with StandardCharsets.UTF_8
Review comment:
Aaaah, I finally found out what causes that. `checkstyleMain.enabled` is set to `false` in the [build.gradle](https://github.com/apache/beam/blob/master/runners/google-cloud-dataflow-java/worker/build.gradle#L118) for the worker project. I set it to `true`, then removed the SuppressionWarning, and when I run `checkstyleMain`, I can see the rule gets triggered. I also didn't know you can set the ID used as the label in, `SuppressionWarnings`, so I fixed that as well!
If you want to test, I think if you set the flag to `true` in the build.gradle file, and comment out the `SuppressionWarnings("ForbidDefaultCharset")`, then run the checkstyleMain, it _should_ work
----------------------------------------------------------------
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:
users@infra.apache.org