-
Notifications
You must be signed in to change notification settings - Fork 344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for multiple integration definitions #235
Support for multiple integration definitions #235
Conversation
77b313e
to
02f5d0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good, just few minor notes but the overall setting is ok!
pkg/client/cmd/run.go
Outdated
|
||
if idx := strings.LastIndexByte(filename, os.PathSeparator); idx > -1 { | ||
codeName = codeName[idx+1:] | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should print a better message saying that if you use multiple sources, the name becomes mandatory. Also, it's dangerous to call the integration "integration" (the old way) when there's no other info, because new integrations override old ones..
pkg/trait/deployment.go
Outdated
@@ -42,7 +43,7 @@ func (d *deploymentTrait) apply(e *environment) error { | |||
return nil | |||
} | |||
|
|||
e.Resources.Add(d.getConfigMapFor(e)) | |||
e.Resources.AddAll(d.getConfigMapFor(e)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configMapsFor
// set env vars needed by the runtime | ||
environment["JAVA_MAIN_CLASS"] = "org.apache.camel.k.jvm.Application" | ||
|
||
// camel-k runtime | ||
environment["CAMEL_K_ROUTES_URI"] = "inline:" + e.Integration.Spec.Source.Content |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate 👍, I didn't like "inline:"!
|
||
for _, s := range e.Integration.Spec.Sources { | ||
meta := metadata.Extract(s) | ||
channels = append(channels, knativeutil.ExtractChannelNames(meta.FromURIs)...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should consider adding to the meta
package some functions to inpect all sources without iterating on them each time.. For the future..
//l = components.get('log') | ||
//l.exchangeFormatter = function(e) { | ||
// return "log - body=" + e.in.body + ", headers=" + e.in.headers | ||
//} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you intend to change this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no leftover of testing
public static Reader resolveInline(String uri) { | ||
if (!uri.startsWith(SCHEME_INLINE)) { | ||
public static Reader resolveEnv(String uri) { | ||
if (!uri.startsWith(Constants.SCHEME_ENV)) { | ||
throw new IllegalArgumentException("The provided content is not inline: " + uri); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
env
74c2061
to
fe80084
Compare
@nicolaferraro done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes #45