[GitHub] [camel-k] lburgazzoli commented on pull request #2519: [discussion] feat: configuration trait

[GitBox <gi...@apache.org>]

lburgazzoli commented on pull request #2519:
URL: https://github.com/apache/camel-k/pull/2519#issuecomment-884811926


   >     1. Right now a property file or other resources attached to an `Integration`, are materialized via `deployment`, `cron` or `knative_service` traits. If we introduce the `configuration` trait we can let this take care of this aspect, although I am not entirely sure if this will have some drawback I am not able to see at the moment (see the changes proposed). It basically means moving the logic that is taking care of it (`e.computeConfigmaps()`) from the 3 trait into the other one. We are introducing some trait dependency (to `configuration` trait), but I don't see this a big concern.
   > 
   >     2. If we're happy with point 1, then we'll need to start moving the logic that is happening now in the `e.computeConfigmaps()` directly in the configuration trait. As an example, in this PR you will see I've done this with the `Properties` parameter which is going to take care of the user properties.
   > 
   >     3. What was done here with `Properties` will have to be extended with all the other user configuration such as `config`, `resource`, ... It seems doable with no big concerns either.
   
   I think that implementing those points is a good opportunity to cleanup the code and make it more consistent so +1 with me 
   
   >     4. `ConfigurationSpec` is widely used in `Integration`, `IntegrationKit` and only in one case in `IntegrationPlatform`. If we proceed with this design change we may be in the situation to extend the trait to the kit as well. Or we can keep it in the kit and the platform and just remove from the integration spec only.
   
   This point is not super clear to me, in particular the reason to remove the trait from the integration.
   
   >     5. The backward compatibility is something we will have to take case as well. I think we can find some way to let both approach (`integration.spec.configspec` and `configuration` trait) coexist, deprecating the first and eventually remove it.
   
   I think that trait.Configure for the configuration trait, should by default attempt to converting the `.spec.config` to its own structure so we can support the two mode for some time but we would swich to the trait internally
   


-- 
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