Re: [PR] feat(api): Kamelet as static resource [camel-k]
squakez commented on PR #4808: URL: https://github.com/apache/camel-k/pull/4808#issuecomment-1750257042 cc @oscerd @gansheer @christophd @lburgazzoli @claudio4j WDYT? Any objection to make the resources static? I think it simplifies the operator with one resource less to consider as I am not aware of any real use case for the Kamelets to be dynamic. Above all, after #4797 where the Kamelets lifecycle will be completely managed at runtime (ie, default Kamelets values). -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(api): Kamelet as static resource [camel-k]
github-actions[bot] commented on PR #4808: URL: https://github.com/apache/camel-k/pull/4808#issuecomment-1750267871 :camel: **Thank you for contributing!** Code Coverage Report :heavy_check_mark: - Coverage unchanged. -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(api): Kamelet as static resource [camel-k]
christophd commented on code in PR #4808:
URL: https://github.com/apache/camel-k/pull/4808#discussion_r1349996836
##
pkg/trait/kamelets.go:
##
@@ -199,39 +184,8 @@ func (t *kameletsTrait) addKamelets(e *Environment) error {
return nil
}
-func (t *kameletsTrait) configureApplicationProperties(e *Environment) error {
Review Comment:
why is this being removed? we need to keep setting the Kamelet default
property values on the application properties or how is this going to be done?
--
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-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat(api): Kamelet as static resource [camel-k]
squakez commented on PR #4808: URL: https://github.com/apache/camel-k/pull/4808#issuecomment-1752590370 > From what I see the Kamelet reconciliation is doing sanity checks on properties and names. Also it computes the Kamelet properties and sets the computed list of properties on the Kamelet status for later usage of default values as application properties. > > I think it is not a bad idea to check the Kamelet when a user changes/adds a Kamelet as a CR. In case we ignore changes and just add the Kamelet as source to the runtime without such a check errors will be reported late in the process at Integration runtime only. Isn't it a good idea to fail fast when the Kamelet is changed and avoid adding it to the Integration when it is broken? > > In general I think having the Kamelets as CRs is a huge benefit Thanks for the feedback. As I mentioned in the description, in order to understand this PR we need to have a look at #4797 first. In that PR we are letting the management of default properties to the runtime. This PR won't remove Kamelets as CR, it will remove its dynamic nature. However the reconciliation today its just making sure that it's a valid name [1]. We can make sure that any validation around names is performed statically when we submit the CR (this is something Kubernetes already does). Mind that this Kamelets mechanics was introduced when Kamelets were not yet a Camel thing, so, likely the reconciliation had a sense back in time. Today it really does nothing that cannot be performed statically. [1] https://github.com/apache/camel-k/pull/4808/files#diff-cdf7e0a6d3147483dcf44a245614fd61ed854341b53c70c9618aac51845ff97aL31 -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(api): Kamelet as static resource [camel-k]
squakez commented on code in PR #4808:
URL: https://github.com/apache/camel-k/pull/4808#discussion_r1350025997
##
pkg/trait/kamelets.go:
##
@@ -199,39 +184,8 @@ func (t *kameletsTrait) addKamelets(e *Environment) error {
return nil
}
-func (t *kameletsTrait) configureApplicationProperties(e *Environment) error {
Review Comment:
They are already managed by the runtime in #4797
--
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-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] feat(api): Kamelet as static resource [camel-k]
gansheer commented on PR #4808: URL: https://github.com/apache/camel-k/pull/4808#issuecomment-1752601230 This is a more general question: does the change from dynamic to static has an impact on how a modification in a Kamelet CR that has an impact on a running integration is handled by the operator ? -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(api): Kamelet as static resource [camel-k]
squakez commented on PR #4808: URL: https://github.com/apache/camel-k/pull/4808#issuecomment-1752609633 > This is a more general question: does the change from dynamic to static has an impact on how a modification in a Kamelet CR that has an impact on a running integration is handled by the operator ? No. The reconciliation only affects the same resource lifecycle. My question for feedback goes in the direction to understand if there is any usage of `.status` by anything external to Camel K. Internally we really do not make use of it. Externally I doubt anybody does, as the only reason why a Kamelet can fail is to have a wrong specification (which would be detected anyhow when submitting the Custom Resource validating its schema). -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(api): Kamelet as static resource [camel-k]
christophd commented on PR #4808: URL: https://github.com/apache/camel-k/pull/4808#issuecomment-1752631016 @squakez yeah I should have looked into PR #4797 first. makes more sense now, thx I had a look into the sanity checks on properties and names that is done on Kamelet reconciliation and it is really quite basic at the moment. I think it only makes sure to not use `source` or `sink` as a Kamelet name and the Kamelet properties should not use `id` as a name. I think we should add this to the documentation instead Also I have had a look into the properties computation done as part of the reconciliation loop and to be honest I have no idea where this should be used in the Kamelet status. Maybe we do a sync with tooling (e.g. Kaoto, ODC) if this is being used but in case the configuration property mechanism is going to change as done in PR #4797 I do not see any use case for that property list in Kamelet status anymore. So finally I am ok with removing the Kamelet reconciliation -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(api): Kamelet as static resource [camel-k]
github-actions[bot] commented on PR #4808: URL: https://github.com/apache/camel-k/pull/4808#issuecomment-1764445426 :camel: **Thank you for contributing!** Code Coverage Report :heavy_check_mark: - Coverage unchanged. -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(api): Kamelet as static resource [camel-k]
github-actions[bot] commented on PR #4808: URL: https://github.com/apache/camel-k/pull/4808#issuecomment-1764474777 :camel: **Thank you for contributing!** Code Coverage Report :heavy_check_mark: - Coverage unchanged. -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] feat(api): Kamelet as static resource [camel-k]
squakez merged PR #4808: URL: https://github.com/apache/camel-k/pull/4808 -- 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-unsubscr...@camel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
