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 template interpretation in config #3782

Merged
merged 4 commits into from
Jun 1, 2018
Merged

Add template interpretation in config #3782

merged 4 commits into from
Jun 1, 2018

Conversation

sbrunner
Copy link
Member

No description provided.

@sbrunner sbrunner added this to the 2.3 milestone May 30, 2018
@sbrunner sbrunner requested a review from pvalsecc May 30, 2018 13:15
@@ -199,7 +199,7 @@ def post(self, command, output_dir, vars_):
"""

if os.name == 'posix':
for file_ in ("docker-run", "docker-compose-run"):
for file_ in ("docker-run", "docker-compose-run", "bin/eval-templates"):
dest = os.path.join(output_dir, "CONST_create_template", file_)
subprocess.check_call(["chmod", "+x", dest])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smells like déjà vu. Please refactor into a function.

find / -name *.tmpl -print | while read file
do
envsubst < ${file} > ${file%.tmpl}
done
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arn't you missing the exec "$@"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To do what? I don't have anything to exec ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not used as an entrypoint?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but in the config container, than he hasn't any other "entry point"...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By not putting an exec in there, you make the life of the dev wanting to debug this image harder... anyway, not a big deal...

#!/bin/bash
set -e

find / -name *.tmpl -print | while read file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that a bit slow to search at the root of the fs?

@sbrunner sbrunner force-pushed the template-config branch 4 times, most recently from eb9539c to 5248df5 Compare May 31, 2018 06:44
@sbrunner sbrunner merged commit 79feadc into 2.3 Jun 1, 2018
@sbrunner sbrunner deleted the template-config branch June 1, 2018 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants