Re: [PR] fix(quarkus): build time properties into file [camel-k]
squakez merged PR #5198: URL: https://github.com/apache/camel-k/pull/5198 -- 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] fix(quarkus): build time properties into file [camel-k]
github-actions[bot] commented on PR #5198: URL: https://github.com/apache/camel-k/pull/5198#issuecomment-1972899383 :heavy_check_mark: Unit test coverage report - coverage increased from 36.4% to 36.8% (**+0.4%**) -- 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] fix(quarkus): build time properties into file [camel-k]
squakez commented on code in PR #5198: URL: https://github.com/apache/camel-k/pull/5198#discussion_r1508761743 ## pkg/builder/quarkus.go: ## @@ -261,6 +201,39 @@ func BuildQuarkusRunnerCommon(ctx context.Context, mc maven.Context, project mav return nil } +func computeApplicationProperties(appPropertiesPath string, applicationProperties map[string]string) error { + f, err := os.OpenFile(appPropertiesPath, os.O_RDWR|os.O_CREATE, 0666) + if err != nil { + return fmt.Errorf("failure while creating application.properties: %w", err) + } + if applicationProperties == nil { + // Default build time properties + applicationProperties = make(map[string]string) + } + // disable quarkus banner + applicationProperties["quarkus.banner.enabled"] = "false" + // required for to resolve data type transformers at runtime with service discovery + // the different Camel runtimes use different resource paths for the service lookup + applicationProperties["quarkus.camel.service.discovery.include-patterns"] = "META-INF/services/org/apache/camel/datatype/converter/*,META-INF/services/org/apache/camel/datatype/transformer/*,META-INF/services/org/apache/camel/transformer/*" + // Workaround to prevent JS runtime errors, see https://github.com/apache/camel-quarkus/issues/5678 + applicationProperties["quarkus.class-loading.parent-first-artifacts"] = "org.graalvm.regex:regex" + defer f.Close() + // Fill with properties coming from user configuration + for k, v := range applicationProperties { + if _, err := f.WriteString(fmt.Sprintf("%s=%s\n", k, v)); err != nil { + return err + } + } + if err := f.Sync(); err != nil { + return err + } + w := bufio.NewWriter(f) Review Comment: Well spotted, thanks. It was a leftover. -- 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] fix(quarkus): build time properties into file [camel-k]
github-actions[bot] commented on PR #5198: URL: https://github.com/apache/camel-k/pull/5198#issuecomment-1972778106 :heavy_check_mark: Unit test coverage report - coverage increased from 36.4% to 36.8% (**+0.4%**) -- 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] fix(quarkus): build time properties into file [camel-k]
squakez commented on code in PR #5198: URL: https://github.com/apache/camel-k/pull/5198#discussion_r1508664654 ## pkg/builder/quarkus.go: ## @@ -163,53 +158,6 @@ func GenerateQuarkusProjectCommon(runtimeVersion string, quarkusVersion string, }, ) - // Add all the properties from the build configuration - p.Properties.AddAll(buildTimeProperties) - - // Quarkus build time properties - buildProperties := make(map[string]string) - - // disable quarkus banner - buildProperties["quarkus.banner.enabled"] = "false" - - // camel-quarkus does route discovery at startup, but we don't want - // this to happen as routes are loaded at runtime and looking for - // routes at build time may try to load camel-k-runtime routes builder - // proxies which in some case may fail. - buildProperties["quarkus.camel.routes-discovery.enabled"] = "false" - - // required for to resolve data type transformers at runtime with service discovery - // the different Camel runtimes use different resource paths for the service lookup - buildProperties["quarkus.camel.service.discovery.include-patterns"] = "META-INF/services/org/apache/camel/datatype/converter/*,META-INF/services/org/apache/camel/datatype/transformer/*,META-INF/services/org/apache/camel/transformer/*" - - // copy all user defined quarkus.camel build time properties to the quarkus-maven-plugin build properties - for key, value := range buildTimeProperties { - if strings.HasPrefix(key, "quarkus.camel.") { - buildProperties[key] = value - } - } - - configuration := v1.PluginProperties{} - configuration.AddProperties("properties", buildProperties) - - // Plugins - p.Build.Plugins = append(p.Build.Plugins, Review Comment: Should be fixed now. Thanks. -- 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] fix(quarkus): build time properties into file [camel-k]
claudio4j commented on code in PR #5198: URL: https://github.com/apache/camel-k/pull/5198#discussion_r1506305752 ## pkg/builder/quarkus.go: ## @@ -261,6 +201,39 @@ func BuildQuarkusRunnerCommon(ctx context.Context, mc maven.Context, project mav return nil } +func computeApplicationProperties(appPropertiesPath string, applicationProperties map[string]string) error { + f, err := os.OpenFile(appPropertiesPath, os.O_RDWR|os.O_CREATE, 0666) + if err != nil { + return fmt.Errorf("failure while creating application.properties: %w", err) + } + if applicationProperties == nil { + // Default build time properties + applicationProperties = make(map[string]string) + } + // disable quarkus banner + applicationProperties["quarkus.banner.enabled"] = "false" + // required for to resolve data type transformers at runtime with service discovery + // the different Camel runtimes use different resource paths for the service lookup + applicationProperties["quarkus.camel.service.discovery.include-patterns"] = "META-INF/services/org/apache/camel/datatype/converter/*,META-INF/services/org/apache/camel/datatype/transformer/*,META-INF/services/org/apache/camel/transformer/*" + // Workaround to prevent JS runtime errors, see https://github.com/apache/camel-quarkus/issues/5678 + applicationProperties["quarkus.class-loading.parent-first-artifacts"] = "org.graalvm.regex:regex" + defer f.Close() + // Fill with properties coming from user configuration + for k, v := range applicationProperties { + if _, err := f.WriteString(fmt.Sprintf("%s=%s\n", k, v)); err != nil { + return err + } + } + if err := f.Sync(); err != nil { + return err + } + w := bufio.NewWriter(f) Review Comment: my knowledge of IO may be outdated, but I am curious why the buffered writer is created after the content was already written and synced to the file ? -- 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] fix(quarkus): build time properties into file [camel-k]
squakez commented on code in PR #5198: URL: https://github.com/apache/camel-k/pull/5198#discussion_r1506294762 ## pkg/builder/quarkus.go: ## @@ -163,53 +158,6 @@ func GenerateQuarkusProjectCommon(runtimeVersion string, quarkusVersion string, }, ) - // Add all the properties from the build configuration - p.Properties.AddAll(buildTimeProperties) - - // Quarkus build time properties - buildProperties := make(map[string]string) - - // disable quarkus banner - buildProperties["quarkus.banner.enabled"] = "false" - - // camel-quarkus does route discovery at startup, but we don't want - // this to happen as routes are loaded at runtime and looking for - // routes at build time may try to load camel-k-runtime routes builder - // proxies which in some case may fail. - buildProperties["quarkus.camel.routes-discovery.enabled"] = "false" - - // required for to resolve data type transformers at runtime with service discovery - // the different Camel runtimes use different resource paths for the service lookup - buildProperties["quarkus.camel.service.discovery.include-patterns"] = "META-INF/services/org/apache/camel/datatype/converter/*,META-INF/services/org/apache/camel/datatype/transformer/*,META-INF/services/org/apache/camel/transformer/*" - - // copy all user defined quarkus.camel build time properties to the quarkus-maven-plugin build properties - for key, value := range buildTimeProperties { - if strings.HasPrefix(key, "quarkus.camel.") { - buildProperties[key] = value - } - } - - configuration := v1.PluginProperties{} - configuration.AddProperties("properties", buildProperties) - - // Plugins - p.Build.Plugins = append(p.Build.Plugins, Review Comment: Yeah, probably not. I see many tests failing. As this was part of #5090 it assumed other changes that are not in at this stage. I'll have a look, thanks. -- 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] fix(quarkus): build time properties into file [camel-k]
github-actions[bot] commented on PR #5198: URL: https://github.com/apache/camel-k/pull/5198#issuecomment-1969065603 :heavy_check_mark: Unit test coverage report - coverage increased from 36.2% to 36.4% (**+0.2%**) -- 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] fix(quarkus): build time properties into file [camel-k]
github-actions[bot] commented on PR #5198: URL: https://github.com/apache/camel-k/pull/5198#issuecomment-1969045284 :heavy_check_mark: Unit test coverage report - coverage increased from 36.2% to 36.4% (**+0.2%**) -- 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
[PR] fix(quarkus): build time properties into file [camel-k]
squakez opened a new pull request, #5198: URL: https://github.com/apache/camel-k/pull/5198 Moving from maven based approach to a file approach in order to let be able to use any character which may be a limitation in pom.xml Closes #5195 **Release Note** ```release-note fix(quarkus): build time properties into file ``` -- 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