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 2022/12/15 15:40:17 UTC

[GitHub] [camel-k] christophd opened a new pull request, #3912: fix(#3844): Add service discovery for Kamelet data type converter

christophd opened a new pull request, #3912:
URL: https://github.com/apache/camel-k/pull/3912

   - Enable service discovery for data type converter resources in order to enable factory finder mechanism when resolving data type implementations
   - Add proper quarkus-maven-plugin build time properties for quarkus.camel.* properties
   - Fixes the way camel-quarkus build time properties are set (set properties on the quarkus-maven-plugin instead of using generic Maven system properties)
   - Explicitly add quarkus.camel.service.discovery.include-patterns for data type converter resources in order to enable lazy loading of Kamelets data type implementations
   
   Relates to [#1980](https://github.com/apache/camel-k/issues/1980)
   
   also see https://github.com/apache/camel-kamelets/tree/main/library/camel-kamelets-utils/src/main/resources/META-INF/services/org/apache/camel/datatype/converter


-- 
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] squakez commented on a diff in pull request #3912: fix(#3844): Add service discovery for Kamelet data type converter

Posted by GitBox <gi...@apache.org>.
squakez commented on code in PR #3912:
URL: https://github.com/apache/camel-k/pull/3912#discussion_r1064718230


##########
pkg/apis/camel/v1/maven_types.go:
##########
@@ -96,4 +96,9 @@ type Server struct {
 	Configuration Properties `xml:"configuration,omitempty" json:"configuration,omitempty"`
 }
 
-type Properties map[string]string

Review Comment:
   I wonder if we could reach the same level of compatibility having `type Properties map[string]interface{}`. In this way, we don't need to include a new type and we neither have to deprecate the old one.



-- 
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] christophd commented on a diff in pull request #3912: fix(#3844): Add service discovery for Kamelet data type converter

Posted by GitBox <gi...@apache.org>.
christophd commented on code in PR #3912:
URL: https://github.com/apache/camel-k/pull/3912#discussion_r1064704762


##########
pkg/apis/camel/v1/maven_types.go:
##########
@@ -96,4 +96,9 @@ type Server struct {
 	Configuration Properties `xml:"configuration,omitempty" json:"configuration,omitempty"`
 }
 
-type Properties map[string]string

Review Comment:
   @squakez I refactored the changes to only use the new plugin properties structure in Maven plugin execution configuration. The other occurrences of properties (e.g. in server configuration) now remain 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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] squakez merged pull request #3912: fix(#3844): Add service discovery for Kamelet data type converter

Posted by GitBox <gi...@apache.org>.
squakez merged PR #3912:
URL: https://github.com/apache/camel-k/pull/3912


-- 
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] christophd commented on a diff in pull request #3912: fix(#3844): Add service discovery for Kamelet data type converter

Posted by GitBox <gi...@apache.org>.
christophd commented on code in PR #3912:
URL: https://github.com/apache/camel-k/pull/3912#discussion_r1064997691


##########
pkg/apis/camel/v1/maven_types.go:
##########
@@ -96,4 +96,9 @@ type Server struct {
 	Configuration Properties `xml:"configuration,omitempty" json:"configuration,omitempty"`
 }
 
-type Properties map[string]string

Review Comment:
   Yeah, I have tried this one, too. It fails in client generator as it is not allowed to use generic interface{} at this point



-- 
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] christophd commented on pull request #3912: fix(#3844): Add service discovery for Kamelet data type converter

Posted by GitBox <gi...@apache.org>.
christophd commented on PR #3912:
URL: https://github.com/apache/camel-k/pull/3912#issuecomment-1359961871

   @squakez I have added some e2e YAKS test with Kamelets using the service discovery mechanism
   
   the failing `install-it` job seems to be non related to this PR


-- 
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] squakez commented on a diff in pull request #3912: fix(#3844): Add service discovery for Kamelet data type converter

Posted by GitBox <gi...@apache.org>.
squakez commented on code in PR #3912:
URL: https://github.com/apache/camel-k/pull/3912#discussion_r1054093419


##########
pkg/apis/camel/v1/maven_types.go:
##########
@@ -96,4 +96,9 @@ type Server struct {
 	Configuration Properties `xml:"configuration,omitempty" json:"configuration,omitempty"`
 }
 
-type Properties map[string]string

Review Comment:
   This change here looks to be breaking compatibility in the API. In order to be performed we should either have a backward compatible field or deprecate the previous one and make it coexist for a couple of versions with the newer.



-- 
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] christophd commented on pull request #3912: fix(#3844): Add service discovery for Kamelet data type converter

Posted by GitBox <gi...@apache.org>.
christophd commented on PR #3912:
URL: https://github.com/apache/camel-k/pull/3912#issuecomment-1375712946

   @squakez please have another look


-- 
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-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org