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

Store integration code in a configmap #55

Merged
merged 1 commit into from Sep 12, 2018

Conversation

Projects
None yet
2 participants
@lburgazzoli
Copy link
Contributor

commented Sep 12, 2018

Fixes #54

@lburgazzoli lburgazzoli requested a review from nicolaferraro Sep 12, 2018

@nicolaferraro
Copy link
Contributor

left a comment

Everything good! One minor comment...

@@ -107,18 +107,6 @@ func createTar(buildDir string, project ProjectDefinition) (string, error) {
return "", err
}

// Environment variables

This comment has been minimized.

Copy link
@nicolaferraro

nicolaferraro Sep 12, 2018

Contributor

This can be useful.. we may need to set some container env in the future. Can you leave it, even if not used by current build mode?

This comment has been minimized.

Copy link
@nicolaferraro

nicolaferraro Sep 12, 2018

Contributor

E.g. I want to disable Jolokia when I don't need it...

This comment has been minimized.

Copy link
@lburgazzoli

lburgazzoli Sep 12, 2018

Author Contributor

Yes will bring them back, but I guess it is better to set them using the deployment so they can be easily inspected so maybe they should be moved to the IntegrationSpec ?

This comment has been minimized.

Copy link
@nicolaferraro

nicolaferraro Sep 12, 2018

Contributor

Mmh, but that can be also a property of the environment, i.e. the "IntegrationContext". And "IntegrationContexts" don't have associated deployments.

@lburgazzoli lburgazzoli force-pushed the lburgazzoli:github-54-cm branch from f9b2035 to 9b6abf9 Sep 12, 2018

@lburgazzoli

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2018

Env restored

@nicolaferraro
Copy link
Contributor

left a comment

Very good!

@nicolaferraro nicolaferraro merged commit c194b5a into apache:master Sep 12, 2018

@lburgazzoli lburgazzoli deleted the lburgazzoli:github-54-cm branch Sep 12, 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.