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

fix(cli): install error message #2112

Merged
merged 1 commit into from
Mar 16, 2021
Merged

Conversation

squakez
Copy link
Contributor

@squakez squakez commented Mar 11, 2021

Printing an error message telling the user OLM is not available if that was selected during installation

Fixes #2105

Release Note

NONE

@astefanutti
Copy link
Member

I wonder if we should do that only when the option is explicitly set, not when defaulted?

@squakez
Copy link
Contributor Author

squakez commented Mar 11, 2021

I wonder if we should do that only when the option is explicitly set, not when defaulted?

I think the usability issue reside in the fact that we choose to default to OLM. If we decided to default to a regular installation, it would be straightforward as we'd call the OLM procedure if that was explicitly required by the user. If we want to leave OLM as default, then we should at least output a warning that we're falling back to a regular installation because OLM is not available. We may think to ask for a confirmation from the user before proceeding ahead with the installation or dropping it.

@squakez
Copy link
Contributor Author

squakez commented Mar 12, 2021

I saw that the integration tests are failing, so I assume that the expected procedure right now is to fallback to a regular installation. However, I think that from usability perspective we should ask a confirmation before proceeding with the installation. I'm logging a separate issue for that.

Printing an error message telling the user OLM is not available if that was selected during installation

Fixes apache#2105
@astefanutti
Copy link
Member

Let's merge this. Yaks tests failures are unrelated.

@astefanutti astefanutti merged commit 8f3e0fb into apache:master Mar 16, 2021
@astefanutti
Copy link
Member

Thanks.

@nicolaferraro nicolaferraro mentioned this pull request Apr 13, 2021
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.

OLM installation fallback to normal installation without warning
3 participants