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

Improve error reporting in case of knative is required but not installed #4237

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

claudio4j
Copy link
Contributor

Fix #3803

The knative check, required the Client object in Environment for various tests.

Release Note

Fix: Improve error reporting in case of knative is required but not installed

Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

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

LGTM, however I wonder if we can reduce that code duplication in the deployment and cron traits moving the logic in the container trait instead:

// Deployment

If not, please consider to create a func with the logic and call it from the 2 places instead.

@squakez squakez merged commit e1e74fc into apache:main Apr 11, 2023
@squakez
Copy link
Contributor

squakez commented Apr 11, 2023

I think it makes sense to backport this one to 1.12 and 1.10.

@claudio4j claudio4j deleted the ups_fix_main_improve_err branch April 11, 2023 11:01
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.

Improve error reporting in case of knative is required but not installed
4 participants