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/10/22 15:35:53 UTC
[GitHub] [camel-k] doru1004 opened a new pull request #1777: Inspect all transitive dependencies of an integration
doru1004 opened a new pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777
<!-- Description -->
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
kind/feature
```
This patch adds more functionality to the `kamel inspect` subcommand. The default behavior of this command consists of outputting the top level dependencies. In this patch we enable the saving of the transitive dependencies for an integration in a specific location on the file system local to where the `kamel inspect` command was invoked. The transitive dependencies consists of all the jar files required for successfully running the integration.
----------------------------------------------------------------
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
[GitHub] [camel-k] lburgazzoli commented on a change in pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r514200232
##########
File path: pkg/cmd/inspect.go
##########
@@ -32,20 +36,30 @@ import (
"github.com/spf13/cobra"
)
+var acceptedDependencyTypes = []string{"bom", "camel", "camel-k", "camel-quarkus", "mvn", "github"}
+
+const defaultDependenciesDirectoryName = "dependencies"
+
func newCmdInspect(rootCmdOptions *RootCmdOptions) (*cobra.Command, *inspectCmdOptions) {
options := inspectCmdOptions{
RootCmdOptions: rootCmdOptions,
}
cmd := cobra.Command{
- Use: "inspect [files to inspect]",
- Short: "Generate dependencies list the given integration files.",
- Long: `Compute and emit the dependencies for a list of integration files.`,
+ Use: "inspect [files to inspect]",
+ Short: "Generate dependencies list given integration files.",
+ Long: `Output dependencies for a list of integration files. By default this command returns the
+top level dependencies only. When --all-dependencies is enabled, the transitive dependencies
+will be generated by calling Maven and then copied into the directory pointed to by the
+--dependencies-directory flag.`,
Review comment:
I'm not very sure about this. Would be useful to just get the list of dependencies without having to copy them on a folder ?
@nicolaferraro what do you think ?
----------------------------------------------------------------
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
[GitHub] [camel-k] nicolaferraro commented on a change in pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
nicolaferraro commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r517482365
##########
File path: pkg/util/defaults/defaults.go
##########
@@ -42,4 +42,7 @@ const (
// ImageName --
ImageName = "docker.io/apache/camel-k"
+
+ // CamelVersion --
+ CamelVersion = "3.6.0"
Review comment:
This is a readonly file, which is generated from Makefile options. We've removed the need to specify the Camel version, because a runtime version implies only one Camel version and one Quarkus version.
Instead of hardcoding it here, you can load the CamelCatalog for the current runtime version (it's embedded in the `/deploy` virtual file system) and retrieve it from the metadata section (https://github.com/apache/camel-k/blob/c79dc9b40ddaa82c38fca405aa484ce60ce5983d/deploy/camel-catalog-1.5.0-quarkus.yaml#L34).
----------------------------------------------------------------
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
[GitHub] [camel-k] nicolaferraro commented on a change in pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
nicolaferraro commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r518097384
##########
File path: pkg/cmd/inspect.go
##########
@@ -155,24 +202,99 @@ func generateCatalog() (*camel.RuntimeCatalog, error) {
Provider: v1.RuntimeProviderMain,
Review comment:
`v1.RuntimeProviderQuarkus` is the default and the only one shortly.
----------------------------------------------------------------
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
[GitHub] [camel-k] nicolaferraro commented on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
nicolaferraro commented on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-722437482
> After changing to use quarkus I'm getting this type errors when inspecting a simple integration:
>
> ```
> [ERROR] 'dependencies.dependency.version' for org.apache.camel.quarkus:camel-quarkus-core:jar is missing. @ line 37, column 17
> [ERROR] 'dependencies.dependency.version' for org.apache.camel.quarkus:camel-quarkus-kafka:jar is missing. @ line 41, column 17
> [ERROR] 'dependencies.dependency.version' for org.apache.camel.quarkus:camel-quarkus-slack:jar is missing. @ line 45, column 17
> ```
>
> Any thoughts?
I think you need to add the quarkus bom as it's done in the builder
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 commented on a change in pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r518100679
##########
File path: pkg/cmd/inspect.go
##########
@@ -182,3 +304,30 @@ func printDependencies(format string, dependecies *strset.Set) error {
}
return nil
}
+
+func getWorkingDirectory() (string, error) {
+ currentDirectory, err := os.Getwd()
+ if err != nil {
+ return "", err
+ }
+
+ return currentDirectory, nil
+}
+
+func createCamelCatalog() (*camel.RuntimeCatalog, error) {
+ // Attempt to reuse existing Camel catalog if one is present.
+ catalog, err := camel.MainCatalog()
Review comment:
Replaced.
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 commented on a change in pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r518100532
##########
File path: pkg/cmd/inspect.go
##########
@@ -155,24 +202,99 @@ func generateCatalog() (*camel.RuntimeCatalog, error) {
Provider: v1.RuntimeProviderMain,
Review comment:
Just changed it.
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 commented on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-722671839
@nicolaferraro I briefly had another error where a warning would be printed when running the maven command. It turned out to be an error on my side as I was using the wrong runtime dependency. Can confirm that no other output than the list of dependencies is being printed.
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 commented on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-722500051
@nicolaferraro I resolved the error. The change involves moving to running with Quarkus specific project building and dependency resolution routines.
----------------------------------------------------------------
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
[GitHub] [camel-k] nicolaferraro commented on a change in pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
nicolaferraro commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r518096791
##########
File path: pkg/cmd/inspect.go
##########
@@ -182,3 +304,30 @@ func printDependencies(format string, dependecies *strset.Set) error {
}
return nil
}
+
+func getWorkingDirectory() (string, error) {
+ currentDirectory, err := os.Getwd()
+ if err != nil {
+ return "", err
+ }
+
+ return currentDirectory, nil
+}
+
+func createCamelCatalog() (*camel.RuntimeCatalog, error) {
+ // Attempt to reuse existing Camel catalog if one is present.
+ catalog, err := camel.MainCatalog()
Review comment:
`camel.DefaultCatalog()` is better. We're removing main.
----------------------------------------------------------------
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
[GitHub] [camel-k] lburgazzoli commented on a change in pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r511794988
##########
File path: pkg/cmd/inspect.go
##########
@@ -32,36 +36,62 @@ import (
"github.com/spf13/cobra"
)
+var acceptedDependencyTypes = []string{"bom", "camel", "camel-k", "camel-quarkus", "mvn", "github"}
+
+const (
+ defaultWorkspaceDirectoryName = "workspace"
+ defaultDependenciesDirectoryName = "dependencies"
+)
+
func newCmdInspect(rootCmdOptions *RootCmdOptions) (*cobra.Command, *inspectCmdOptions) {
options := inspectCmdOptions{
RootCmdOptions: rootCmdOptions,
}
cmd := cobra.Command{
- Use: "inspect [files to inspect]",
- Short: "Generate dependencies list the given integration files.",
- Long: `Compute and emit the dependencies for a list of integration files.`,
+ Use: "inspect [files to inspect]",
+ Short: "Generate dependencies list given integration files.",
+ Long: `Output dependencies for a list of integration files. By default this command returns the
+top level dependencies only. When --all-dependencies is enabled, the transitive dependencies
+will be generated by calling Maven and then copied into the directory pointed to by the
+--dependencies-directory flag.`,
PreRunE: decode(&options),
RunE: func(_ *cobra.Command, args []string) error {
if err := options.validate(args); err != nil {
return err
}
+ if err := options.initialize(args); err != nil {
+ return err
+ }
if err := options.run(args); err != nil {
fmt.Println(err.Error())
}
return nil
},
+ Annotations: map[string]string{
+ offlineCommandLabel: "true",
+ },
}
+ cmd.Flags().Bool("all-dependencies", false, "Compute transitive dependencies and move them to directory pointed to by the --dependencies-directory flag.")
+ cmd.Flags().StringArrayP("additional-dependencies", "d", nil, `Comma-separated lists of additional top-level dependencies with the format:
Review comment:
I don't think they necessarily be copmma separated but you can also repeat the flag
##########
File path: pkg/builder/builder_steps.go
##########
@@ -140,7 +141,17 @@ func generateProjectSettings(ctx *Context) error {
func injectDependencies(ctx *Context) error {
// Add dependencies from build
- for _, d := range ctx.Build.Dependencies {
+ return InjectDependenciesCommon(&ctx.Maven.Project, ctx.Build.Dependencies, ctx.Catalog)
+}
+
+// InjectDependenciesCommon --
+func InjectDependenciesCommon(
Review comment:
We should avoid this `*Common` suffix, I'd rather move those functions to a dedicated package or rethink the names used for the steps
##########
File path: pkg/cmd/inspect.go
##########
@@ -32,36 +36,62 @@ import (
"github.com/spf13/cobra"
)
+var acceptedDependencyTypes = []string{"bom", "camel", "camel-k", "camel-quarkus", "mvn", "github"}
+
+const (
+ defaultWorkspaceDirectoryName = "workspace"
+ defaultDependenciesDirectoryName = "dependencies"
+)
+
func newCmdInspect(rootCmdOptions *RootCmdOptions) (*cobra.Command, *inspectCmdOptions) {
options := inspectCmdOptions{
RootCmdOptions: rootCmdOptions,
}
cmd := cobra.Command{
- Use: "inspect [files to inspect]",
- Short: "Generate dependencies list the given integration files.",
- Long: `Compute and emit the dependencies for a list of integration files.`,
+ Use: "inspect [files to inspect]",
+ Short: "Generate dependencies list given integration files.",
+ Long: `Output dependencies for a list of integration files. By default this command returns the
+top level dependencies only. When --all-dependencies is enabled, the transitive dependencies
+will be generated by calling Maven and then copied into the directory pointed to by the
+--dependencies-directory flag.`,
PreRunE: decode(&options),
RunE: func(_ *cobra.Command, args []string) error {
if err := options.validate(args); err != nil {
return err
}
+ if err := options.initialize(args); err != nil {
+ return err
+ }
if err := options.run(args); err != nil {
fmt.Println(err.Error())
}
return nil
},
+ Annotations: map[string]string{
+ offlineCommandLabel: "true",
+ },
}
+ cmd.Flags().Bool("all-dependencies", false, "Compute transitive dependencies and move them to directory pointed to by the --dependencies-directory flag.")
+ cmd.Flags().StringArrayP("additional-dependencies", "d", nil, `Comma-separated lists of additional top-level dependencies with the format:
+<type>:<dependency-name>
+where <type> is one of {`+strings.Join(acceptedDependencyTypes, "|")+`}.`)
+ cmd.Flags().String("workspace", "", "Workspace directory. Default: <kamel-invocation-directory>/workspace")
Review comment:
I'm not very sure we want to expose this "workspace" concept to the user as in general camel-k hides all the project related stuffs
##########
File path: pkg/cmd/inspect.go
##########
@@ -32,36 +36,62 @@ import (
"github.com/spf13/cobra"
)
+var acceptedDependencyTypes = []string{"bom", "camel", "camel-k", "camel-quarkus", "mvn", "github"}
+
+const (
+ defaultWorkspaceDirectoryName = "workspace"
+ defaultDependenciesDirectoryName = "dependencies"
+)
+
func newCmdInspect(rootCmdOptions *RootCmdOptions) (*cobra.Command, *inspectCmdOptions) {
options := inspectCmdOptions{
RootCmdOptions: rootCmdOptions,
}
cmd := cobra.Command{
- Use: "inspect [files to inspect]",
- Short: "Generate dependencies list the given integration files.",
- Long: `Compute and emit the dependencies for a list of integration files.`,
+ Use: "inspect [files to inspect]",
+ Short: "Generate dependencies list given integration files.",
+ Long: `Output dependencies for a list of integration files. By default this command returns the
+top level dependencies only. When --all-dependencies is enabled, the transitive dependencies
+will be generated by calling Maven and then copied into the directory pointed to by the
+--dependencies-directory flag.`,
PreRunE: decode(&options),
RunE: func(_ *cobra.Command, args []string) error {
if err := options.validate(args); err != nil {
return err
}
+ if err := options.initialize(args); err != nil {
+ return err
+ }
if err := options.run(args); err != nil {
fmt.Println(err.Error())
}
return nil
},
+ Annotations: map[string]string{
+ offlineCommandLabel: "true",
+ },
}
+ cmd.Flags().Bool("all-dependencies", false, "Compute transitive dependencies and move them to directory pointed to by the --dependencies-directory flag.")
+ cmd.Flags().StringArrayP("additional-dependencies", "d", nil, `Comma-separated lists of additional top-level dependencies with the format:
Review comment:
maybe it is better to keep it as `--dependency` as it is for the run command
##########
File path: pkg/cmd/inspect.go
##########
@@ -86,32 +116,110 @@ func (command *inspectCmdOptions) validate(args []string) error {
}
}
+ // Validate list of additional dependencies i.e. make sure that each dependency has
+ // a valid type.
+ if command.AdditionalDependencies != nil {
+ for _, additionalDependency := range command.AdditionalDependencies {
+ additionalDependencies := strings.Split(additionalDependency, ",")
+
+ for _, dependency := range additionalDependencies {
+ dependencyComponents := strings.Split(dependency, ":")
+
+ TypeIsValid := false
+ for _, dependencyType := range acceptedDependencyTypes {
+ if dependencyType == dependencyComponents[0] {
+ TypeIsValid = true
+ }
+ }
+
+ if !TypeIsValid {
+ return errors.New("Unexpected type for user-provided dependency: " + dependency + ", check command usage for valid format.")
+ }
+ }
+ }
+ }
+
+ // If provided, ensure that that the dependencies directory exists.
+ if command.DependenciesDirectory != "" {
+ dependenciesDirectoryExists, err := util.DirectoryExists(command.DependenciesDirectory)
+ // Report any error.
+ if err != nil {
+ return err
+ }
+
+ // Signal file not found.
+ if !dependenciesDirectoryExists {
+ return errors.New("input file " + command.DependenciesDirectory + " file does not exist")
+ }
+ }
+
+ return nil
+}
+
+func (command *inspectCmdOptions) initialize(args []string) error {
+ // If --all-dependencies flag is set the workspace and dependencies directories need to have
+ // valid values. If not provided on the command line, the values need to be initialized with
+ // their default values.
+ if command.AllDependencies {
+ // Create workspace directory to hold all intermediate files.
+ err := createAndSetWorkspaceDirectory(command)
+ if err != nil {
+ return err
+ }
+
+ // Move the integration dependecies to the dependencies directory.
+ err = createAndSetDependenciesDirectory(command)
+ if err != nil {
+ return err
+ }
+ }
return nil
}
func (command *inspectCmdOptions) run(args []string) error {
- // Attempt to reuse existing Camel catalog if one is present.
- catalog, err := camel.MainCatalog()
+ // Fetch existing catalog or create new one if one does not already exist.
+ catalog, err := createCamelCatalog()
+
+ // Output top-level dependencies.
+ outputTopLevel := !command.AllDependencies
Review comment:
I think transitive dependencies should not be handled in a different manner than the top level one so they should be included in the output as well.
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 commented on a change in pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r512944750
##########
File path: pkg/cmd/inspect.go
##########
@@ -86,32 +116,110 @@ func (command *inspectCmdOptions) validate(args []string) error {
}
}
+ // Validate list of additional dependencies i.e. make sure that each dependency has
+ // a valid type.
+ if command.AdditionalDependencies != nil {
+ for _, additionalDependency := range command.AdditionalDependencies {
+ additionalDependencies := strings.Split(additionalDependency, ",")
+
+ for _, dependency := range additionalDependencies {
+ dependencyComponents := strings.Split(dependency, ":")
+
+ TypeIsValid := false
+ for _, dependencyType := range acceptedDependencyTypes {
+ if dependencyType == dependencyComponents[0] {
+ TypeIsValid = true
+ }
+ }
+
+ if !TypeIsValid {
+ return errors.New("Unexpected type for user-provided dependency: " + dependency + ", check command usage for valid format.")
+ }
+ }
+ }
+ }
+
+ // If provided, ensure that that the dependencies directory exists.
+ if command.DependenciesDirectory != "" {
+ dependenciesDirectoryExists, err := util.DirectoryExists(command.DependenciesDirectory)
+ // Report any error.
+ if err != nil {
+ return err
+ }
+
+ // Signal file not found.
+ if !dependenciesDirectoryExists {
+ return errors.New("input file " + command.DependenciesDirectory + " file does not exist")
+ }
+ }
+
+ return nil
+}
+
+func (command *inspectCmdOptions) initialize(args []string) error {
+ // If --all-dependencies flag is set the workspace and dependencies directories need to have
+ // valid values. If not provided on the command line, the values need to be initialized with
+ // their default values.
+ if command.AllDependencies {
+ // Create workspace directory to hold all intermediate files.
+ err := createAndSetWorkspaceDirectory(command)
+ if err != nil {
+ return err
+ }
+
+ // Move the integration dependecies to the dependencies directory.
+ err = createAndSetDependenciesDirectory(command)
+ if err != nil {
+ return err
+ }
+ }
return nil
}
func (command *inspectCmdOptions) run(args []string) error {
- // Attempt to reuse existing Camel catalog if one is present.
- catalog, err := camel.MainCatalog()
+ // Fetch existing catalog or create new one if one does not already exist.
+ catalog, err := createCamelCatalog()
+
+ // Output top-level dependencies.
+ outputTopLevel := !command.AllDependencies
Review comment:
I have included them in the output if the format flag is enabled. When outputting transitive dependencies they are dumped in a folder accessible by the user. Top level dependencies should not be included in the output if transitive dependencies are enabled.
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 commented on a change in pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r517526583
##########
File path: pkg/util/defaults/defaults.go
##########
@@ -42,4 +42,7 @@ const (
// ImageName --
ImageName = "docker.io/apache/camel-k"
+
+ // CamelVersion --
+ CamelVersion = "3.6.0"
Review comment:
Done, thanks for the feedback.
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 commented on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-722414440
> I.e. the maven part should be probably removed from the output. Wdyt?
The maven needs to be run to fetch the dependencies. I'm guessing your concern is regarding whether the output of that command is shown or not. Is that correct?
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 edited a comment on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 edited a comment on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-722632007
----------------------------------------------------------------
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
[GitHub] [camel-k] nicolaferraro commented on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
nicolaferraro commented on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-722412453
> That's cool @doru1004 .
>
> The only thing strange I see is that the output looks like:
>
> ```
> [nferraro@localhost camel-k]$ kamel inspect examples/simple.groovy --all-dependencies --output=yaml
> [INFO] Scanning for projects...
> [INFO]
> [INFO] ---------< org.apache.camel.k.integration:camel-k-integration >---------
> [INFO] Building camel-k-integration 1.3.0-SNAPSHOT
> [INFO] --------------------------------[ jar ]---------------------------------
> [INFO]
> [INFO] --- camel-k-maven-plugin:1.5.0:generate-dependency-list (default-cli) @ camel-k-integration ---
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 0.856 s
> [INFO] Finished at: 2020-11-05T15:24:46+01:00
> [INFO] ------------------------------------------------------------------------
> dependencies:
> - /home/nferraro/.m2/repository/org/apache/camel/k/camel-k-loader-groovy/1.5.0/camel-k-loader-groovy-1.5.0.jar
> - /home/nferraro/.m2/repository/org/apache/camel/k/camel-k-runtime-core/1.5.0/camel-k-runtime-core-1.5.0.jar
> ```
>
> I.e. the maven part should be probably removed from the output. Wdyt?
Or at least shown only when there's an error
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 removed a comment on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 removed a comment on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-722632007
I managed to prevent the output of the mvn command from being emitted. There is still an issue with one of the warnings being emitted:
```
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/Users/dorubercea/.m2/repository/org/jboss/slf4j/slf4j-jboss-logging/1.2.0.Final/slf4j-jboss-logging-1.2.0.Final.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/Users/dorubercea/.m2/repository/org/apache/logging/log4j/log4j-slf4j-impl/2.13.3/log4j-slf4j-impl-2.13.3.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual binding is of type [org.jboss.slf4j.JBossLoggerFactory]
```
I'm not sure how to get rid of this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] doru1004 commented on a change in pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r512983038
##########
File path: pkg/builder/builder_steps.go
##########
@@ -140,7 +141,17 @@ func generateProjectSettings(ctx *Context) error {
func injectDependencies(ctx *Context) error {
// Add dependencies from build
- for _, d := range ctx.Build.Dependencies {
+ return InjectDependenciesCommon(&ctx.Maven.Project, ctx.Build.Dependencies, ctx.Catalog)
+}
+
+// InjectDependenciesCommon --
+func InjectDependenciesCommon(
Review comment:
I think this function belongs in a utils file rather than here. It performs a role that is independent of anything other than a Maven project + Dependencies + Catalog which don't necessarily have to be inside the builder. I've moved the function to a new file under utils/camel. I have also moved the function which sanitizes integration dependencies.
----------------------------------------------------------------
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
[GitHub] [camel-k] nicolaferraro merged pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
nicolaferraro merged pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 commented on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-720618232
@lburgazzoli I have modified the patch to only print transitive dependencies. Please let me know if you have any further comments.
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 commented on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-722422978
After changing to use quarkus I'm getting this type errors when inspecting a simple integration:
```
[ERROR] 'dependencies.dependency.version' for org.apache.camel.quarkus:camel-quarkus-core:jar is missing. @ line 37, column 17
[ERROR] 'dependencies.dependency.version' for org.apache.camel.quarkus:camel-quarkus-kafka:jar is missing. @ line 41, column 17
[ERROR] 'dependencies.dependency.version' for org.apache.camel.quarkus:camel-quarkus-slack:jar is missing. @ line 45, column 17
```
Any thoughts?
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 commented on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-722632007
I managed to prevent the output of the mvn command from being emitted. This is still an issue with one of the warning being emitted:
```
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/Users/dorubercea/.m2/repository/org/jboss/slf4j/slf4j-jboss-logging/1.2.0.Final/slf4j-jboss-logging-1.2.0.Final.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/Users/dorubercea/.m2/repository/org/apache/logging/log4j/log4j-slf4j-impl/2.13.3/log4j-slf4j-impl-2.13.3.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual binding is of type [org.jboss.slf4j.JBossLoggerFactory]
```
I'm not sure how to get rid of this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] nicolaferraro commented on a change in pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
nicolaferraro commented on a change in pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#discussion_r518096246
##########
File path: pkg/cmd/inspect.go
##########
@@ -89,32 +105,68 @@ func (command *inspectCmdOptions) validate(args []string) error {
}
}
+ // Validate list of additional dependencies i.e. make sure that each dependency has
+ // a valid type.
+ if command.AdditionalDependencies != nil {
+ for _, additionalDependency := range command.AdditionalDependencies {
+ dependencyComponents := strings.Split(additionalDependency, ":")
+
+ TypeIsValid := false
+ for _, dependencyType := range acceptedDependencyTypes {
+ if dependencyType == dependencyComponents[0] {
+ TypeIsValid = true
+ }
+ }
+
+ if !TypeIsValid {
+ return errors.New("Unexpected type for user-provided dependency: " + additionalDependency + ", check command usage for valid format.")
+ }
+
+ }
+ }
+
return nil
}
func (command *inspectCmdOptions) run(args []string) error {
- // Attempt to reuse existing Camel catalog if one is present.
- catalog, err := camel.MainCatalog()
Review comment:
`camel.DefaultCatalog()` is better. We're removing main.
----------------------------------------------------------------
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
[GitHub] [camel-k] doru1004 commented on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
doru1004 commented on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-717993120
@lburgazzoli I think I have addressed all the comments. Please let me know what you think. I've made some changes to where the functions that compute the integration dependencies and their sanitization are defined. Let me know if you agree with the move.
----------------------------------------------------------------
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
[GitHub] [camel-k] nicolaferraro commented on pull request #1777: Inspect all transitive dependencies of an integration
Posted by GitBox <gi...@apache.org>.
nicolaferraro commented on pull request #1777:
URL: https://github.com/apache/camel-k/pull/1777#issuecomment-722417826
> > I.e. the maven part should be probably removed from the output. Wdyt?
>
> The maven needs to be run to fetch the dependencies. I'm guessing your concern is regarding whether the output of that command is shown or not. Is that correct?
Yes, it's only about the output. I imagine that external tools that are going to process the output of that command may get confused.
----------------------------------------------------------------
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