Skip to content
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

add resources to an integration #300

Merged
merged 3 commits into from Dec 20, 2018

Conversation

Projects
None yet
4 participants
@lburgazzoli
Copy link
Contributor

commented Dec 17, 2018

This implementation has required a larger refactor than expected
but now the code is cleaner and some more things have been moved
out from the builder and moved to traits phase

Fixes #241

@oscerd

oscerd approved these changes Dec 17, 2018

@nicolaferraro
Copy link
Contributor

left a comment

I think the Knative part is missing..

@@ -147,8 +147,7 @@ type Context struct {
Artifacts []v1alpha1.Artifact
SelectedArtifacts []v1alpha1.Artifact
Archive string
ComputeClasspath bool
MainClass string
ContextFiler func(integrationContext *v1alpha1.IntegrationContext) bool

This comment has been minimized.

Copy link
@nicolaferraro

nicolaferraro Dec 18, 2018

Contributor

Typo

@lburgazzoli

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

I've fixed the typo but for knative we have an issue as we can expose resources as env var but then we can't use standard java resource loading mechanism so I guess that for knative we should always generate a new container image until they find a solution to inject resources to an image.

@lburgazzoli

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

@nicolaferraro

ok, so I've added support for looking up resources using env vars. using camel dsl it should look like:

from('timer:resources')
    .routeId('resources')
    .setBody()
        .simple("resource:platform:resources-data.txt")
    .log('file content is: ${body}')

do you mind having a look ?

@lburgazzoli lburgazzoli force-pushed the lburgazzoli:github-241 branch from 4c5f969 to 2d60b80 Dec 18, 2018

@davsclaus

This comment has been minimized.

Copy link
Contributor

commented Dec 18, 2018

I think we can also add support for env: to Apache Camel's ResourceHelper and maybe also JVM system properties, then we got all of them covered (fingers crossed).

@lburgazzoli

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

yes we should.

The main problem here is that there are differences between knative and other environments so the platform protocol has to be smart. In this case is not so smart :) as it simply check for the presence of a file on the file system, in the class-path and finally it checks the env, but it works.

Maybe we can introduce the "platform" protocol also in Apache Camel and have a way to configure what real protocol(s) it has to use under the hoods.

I think that lot of thing will be moved in standard Camel utilities once the camel-k runtime will be moved to Apache Camel.

@nicolaferraro
Copy link
Contributor

left a comment

Nice!

@nicolaferraro nicolaferraro merged commit 7da47e3 into apache:master Dec 20, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@lburgazzoli lburgazzoli deleted the lburgazzoli:github-241 branch Dec 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.