You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2020/02/11 15:16:14 UTC
[GitHub] [camel-quarkus] johnpoth opened a new issue #699: Remove
UnbannedReflectiveBuildItem + policy extension ?
johnpoth opened a new issue #699: Remove UnbannedReflectiveBuildItem + policy extension ?
URL: https://github.com/apache/camel-quarkus/issues/699
When a component uses the `camel-api-component-maven-plugin` (`olingo4`, `box`, `fhir`, `zendesk` etc...) one has to register the Configuration classes for reflection using `ReflectiveClassBuildItem`.
With the Policy extension, one also has to register the same classes with `UnbannedReflectiveBuildItem` or else the build fails.
Since these are valid uses of `ReflectiveClassBuildItem` I really don't think the build should fail.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [camel-quarkus] johnpoth commented on issue #699: Remove
UnbannedReflectiveBuildItem + policy extension ?
Posted by GitBox <gi...@apache.org>.
johnpoth commented on issue #699: Remove UnbannedReflectiveBuildItem + policy extension ?
URL: https://github.com/apache/camel-quarkus/issues/699#issuecomment-584702364
I'm not sure I understand, some classes annotated with `@org.apache.camel.spi.UriParams` do need reflection, so why make the build fail ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [camel-quarkus] johnpoth commented on issue #699: Remove
UnbannedReflectiveBuildItem + policy extension ?
Posted by GitBox <gi...@apache.org>.
johnpoth commented on issue #699: Remove UnbannedReflectiveBuildItem + policy extension ?
URL: https://github.com/apache/camel-quarkus/issues/699#issuecomment-584706780
I agree with @lburgazzoli, @oscerd
Build failures should really only occur in critical situations. IMO this hurts the overall developer experience 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli commented on issue #699: Remove
UnbannedReflectiveBuildItem + policy extension ?
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on issue #699: Remove UnbannedReflectiveBuildItem + policy extension ?
URL: https://github.com/apache/camel-quarkus/issues/699#issuecomment-584737142
I think the warning is the right thing as it also avoid to keep a list of white listed classes (that needs to be maintained)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #699: Remove
UnbannedReflectiveBuildItem + policy extension ?
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #699: Remove UnbannedReflectiveBuildItem + policy extension ?
URL: https://github.com/apache/camel-quarkus/issues/699#issuecomment-584699228
We currently ban classes annotated with `@org.apache.camel.spi.UriParams` that actually should not need reflection, because there are means on the Camel site how to avoid it. If those means do not work, one should file a Camel issue or even better fix it on the Camel side. Unbanning should be seen only as a temporary measure to make things work. Registering for reflection comes at some cost (image size, runtime efficiency) and we want to avoid it whereever possible.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [camel-quarkus] lburgazzoli commented on issue #699: Remove
UnbannedReflectiveBuildItem + policy extension ?
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on issue #699: Remove UnbannedReflectiveBuildItem + policy extension ?
URL: https://github.com/apache/camel-quarkus/issues/699#issuecomment-584701996
I'd say that we should trust a little extensions developers and thus evaluation should be done mainly through the PR review process.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [camel-quarkus] oscerd commented on issue #699: Remove
UnbannedReflectiveBuildItem + policy extension ?
Posted by GitBox <gi...@apache.org>.
oscerd commented on issue #699: Remove UnbannedReflectiveBuildItem + policy extension ?
URL: https://github.com/apache/camel-quarkus/issues/699#issuecomment-584703871
Because on the camel side there were efforts to avoid reflection and there are multiple commits where this is visible on each component. It wasn't possible to do this on all the components btw, so I agree the build should not fail.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [camel-quarkus] jamesnetherton commented on issue #699: Remove
UnbannedReflectiveBuildItem + policy extension ?
Posted by GitBox <gi...@apache.org>.
jamesnetherton commented on issue #699: Remove UnbannedReflectiveBuildItem + policy extension ?
URL: https://github.com/apache/camel-quarkus/issues/699#issuecomment-584735056
As a compromise, can we not just log a message at `WARN` or `DEBUG` level if `UriParams` stuff is registered for reflection? At least that'll give an easy way to track stuff down for investigation.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #699: Remove
UnbannedReflectiveBuildItem + policy extension ?
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #699: Remove UnbannedReflectiveBuildItem + policy extension ?
URL: https://github.com/apache/camel-quarkus/issues/699#issuecomment-584730522
I am sorry for the inconvenience, but I still think that registering a class for reflection that belongs to a well known group that is expected not to need it, should not go without checking it properly.
Speaking for me, I do not feel able to eval during the review by pure sight whether some Camel class has some specific annotation. I think the policy does some useful work there. If we really want to tackle this, we should keep the policy.
Going through tens of extensions manually afterwards to check whether we register for reflection only what is necessary is a lot of work, that I did twice and I would not wish anybody to that again.
However, if we actually do not have the ambition to do this properly, I won't be against removing the policy.
By no means this inconvenience can impact Camel Quarkus application developers unless they consciously depend on the policy extension. This is only the case in our integration tests. Failing is the right thing to do there because what does not fail the build gets ignored.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on issue #699: Remove
UnbannedReflectiveBuildItem + policy extension ?
Posted by GitBox <gi...@apache.org>.
ppalaga commented on issue #699: Remove UnbannedReflectiveBuildItem + policy extension ?
URL: https://github.com/apache/camel-quarkus/issues/699#issuecomment-584740566
OK, accepting that the policy is not seen as useful by the majority and thus also switching to passive mode for the solving of the underlying problem.
----------------------------------------------------------------
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
With regards,
Apache Git Services