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

Don't fail on template missing parameters #1049

Merged
merged 1 commit into from Sep 9, 2020

Conversation

apahim
Copy link
Contributor

@apahim apahim commented Sep 8, 2020

The parameters are no really mandatory. Let's not fail when they are not
present in the template.

Signed-off-by: Amador Pahim apahim@redhat.com

@apahim apahim requested a review from maorfr September 8, 2020 15:32
@maorfr
Copy link
Contributor

maorfr commented Sep 8, 2020

Doesn't oc process fails when there are no parameters?

@maorfr
Copy link
Contributor

maorfr commented Sep 8, 2020

Also look at the history. This error was added due to a problem we had...

@apahim
Copy link
Contributor Author

apahim commented Sep 8, 2020

Doesn't oc process fails when there are no parameters?

no

Also look at the history. This error was added due to a problem we had...

history says that much... do you know what was the error? maybe it was you trying to loop a NoneType?

@maorfr
Copy link
Contributor

maorfr commented Sep 9, 2020

I only found this PR: #864. I didn't link any issues in it, but i believe it was about trying to look into the parameters which didn't exist.
I think your approach works just as well.

Copy link
Contributor

@maorfr maorfr left a comment

Choose a reason for hiding this comment

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

LGTM

@apahim
Copy link
Contributor Author

apahim commented Sep 9, 2020

thanks!

The parameters are no really mandatory. Let's not fail when they are not
present in the template.

Signed-off-by: Amador Pahim <apahim@redhat.com>
@apahim apahim merged commit b5392cc into app-sre:master Sep 9, 2020
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.

None yet

2 participants