jamesnetherton commented on code in PR #9081: URL: https://github.com/apache/camel-quarkus/pull/9081#discussion_r3905050863
########## docs/modules/ROOT/pages/reference/extensions/twitter.adoc: ########## @@ -53,3 +53,34 @@ endif::[] This extension auto-enables SSL support in native mode. Hence you do not need to add `quarkus.ssl.native=true` to your `application.properties` yourself. See also https://quarkus.io/guides/native-and-ssl[Quarkus SSL guide]. + +[id="extensions-twitter-camel-quarkus-limitations"] +== Camel Quarkus limitations Review Comment: This page is generated ("Do not edit directly!"), and this section is in the wrong position — the template emits `Camel Quarkus limitations` before `SSL in native mode`. Regenerating will move it and CI's uncommitted-changes check will fail. Run `./mvnw -pl extensions/twitter/deployment process-classes` and commit the output instead of editing this file directly. ########## extensions/twitter/runtime/src/main/doc/limitations.adoc: ########## @@ -0,0 +1,27 @@ +The Twitter Java client parses timestamps with `Locale.ENGLISH` and expects timezone UTC. Review Comment: twitter4j uses `Locale.US`, not `Locale.ENGLISH` (`ParseUtil` in both 4.0.7 and 4.1.2) — this is also how #2564 words it. Worth being precise, since `Locale.ENGLISH` is `en` whereas the property recommended just below is `en-US`. Also "and expects timezone UTC" should go — see the note on the timezone block below. ########## extensions/twitter/runtime/src/main/doc/limitations.adoc: ########## @@ -0,0 +1,27 @@ +The Twitter Java client parses timestamps with `Locale.ENGLISH` and expects timezone UTC. +In native mode, GraalVM includes only the build machine locale by default. A non-English locale +(for example French) then fails to parse Twitter API responses. + +Set the default locale to `en-US` in `application.properties`: + +[source,properties] +---- +quarkus.default-locale=en-US +---- + +You can also pin language and country on the native image: + +[source,properties] +---- +quarkus.native.user-language=en +quarkus.native.user-country=US +---- + Review Comment: ```suggestion ``` These are from a 2021 comment on #2564 and were valid then, but they have since been deprecated in favour of `quarkus.default-locale` and removed — they are absent from `quarkus-core-deployment-3.39.0.jar` entirely (`pom.xml:68`). Anyone following this gets an "Unrecognized configuration key" warning and no behaviour change. The `quarkus.default-locale=en-US` block above is sufficient; `quarkus.locales` covers the multi-locale case if it ever comes up. ########## extensions/twitter/runtime/src/main/doc/limitations.adoc: ########## @@ -0,0 +1,27 @@ +The Twitter Java client parses timestamps with `Locale.ENGLISH` and expects timezone UTC. +In native mode, GraalVM includes only the build machine locale by default. A non-English locale +(for example French) then fails to parse Twitter API responses. + +Set the default locale to `en-US` in `application.properties`: + +[source,properties] +---- +quarkus.default-locale=en-US +---- + +You can also pin language and country on the native image: + +[source,properties] +---- +quarkus.native.user-language=en +quarkus.native.user-country=US +---- + +Pin the timezone to UTC for the native build: + +[source,properties] +---- +quarkus.native.additional-build-args=-J-Duser.timezone=UTC +---- + Review Comment: ```suggestion ``` I would drop the timezone guidance rather than document it. It was never confirmed on #2564 — the tests could not be run for want of credentials — and it does not hold now: `-J-D...` sets the property on the image-builder JVM rather than the executable, and twitter4j parses with explicit-offset patterns (4.1.2) or an explicitly UTC-set formatter (4.0.7), so it never reads the JVM default zone. ########## integration-tests/twitter/src/main/resources/application.properties: ########## @@ -43,3 +43,11 @@ camel.component.twitter-directmessage.consumerKey={{env:TWITTER_CONSUMER_KEY}} camel.component.twitter-directmessage.consumerSecret={{env:TWITTER_CONSUMER_SECRET}} camel.component.twitter-directmessage.accessToken={{env:TWITTER_ACCESS_TOKEN}} camel.component.twitter-directmessage.accessTokenSecret={{env:TWITTER_ACCESS_TOKEN_SECRET}} + +# +# Native image: Twitter4J parses dates with Locale.ENGLISH and timezone UTC +# +quarkus.default-locale=en-US +quarkus.native.user-language=en +quarkus.native.user-country=US +quarkus.native.additional-build-args=-J-Duser.timezone=UTC Review Comment: ```suggestion ``` This does not affect the native executable's default timezone (`-J-D` targets the builder JVM), and twitter4j does not depend on the JVM default zone. Setting `quarkus.native.additional-build-args` also claims the property for this whole module — if a build arg is genuinely needed later, use the `-append` variant as `integration-tests/milo/pom.xml:85` does. ########## integration-tests/twitter/src/main/resources/application.properties: ########## @@ -43,3 +43,11 @@ camel.component.twitter-directmessage.consumerKey={{env:TWITTER_CONSUMER_KEY}} camel.component.twitter-directmessage.consumerSecret={{env:TWITTER_CONSUMER_SECRET}} camel.component.twitter-directmessage.accessToken={{env:TWITTER_ACCESS_TOKEN}} camel.component.twitter-directmessage.accessTokenSecret={{env:TWITTER_ACCESS_TOKEN_SECRET}} + +# +# Native image: Twitter4J parses dates with Locale.ENGLISH and timezone UTC +# +quarkus.default-locale=en-US +quarkus.native.user-language=en +quarkus.native.user-country=US Review Comment: ```suggestion ``` Same as the doc — unrecognized under Quarkus 3.39, so these only add build warnings. `quarkus.default-locale=en-US` on line 50 is the whole fix. Compare `integration-tests/minio/src/main/resources/application.properties:19`, which sets only the default locale. ########## integration-tests/twitter/README.adoc: ########## @@ -17,3 +17,6 @@ export TWITTER_USER_NAME=my-twitter-username ---- + +Native tests need `Locale.ENGLISH` and timezone UTC, otherwise Twitter API date parsing fails. +The test `application.properties` already sets `quarkus.default-locale=en-US` and UTC for the native image. Review Comment: "and UTC for the native image" is not accurate — the timezone arg does not reach the native executable. Once the UTC line is dropped from `application.properties`, this sentence should say only that the default locale is set. Also `Locale.ENGLISH` on line 21 should be `Locale.US`. -- 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]
