Replace wif models and client with sdk

openshift-online / ocm-cli

CLI for the Red Hat OpenShift Cluster Manager

Apache License 2.0

76 stars 137 forks source link

Replace wif models and client with sdk #643

Closed JakobGray closed 2 months ago

JakobGray commented 2 months ago

This PR will

Update to the latest version of the ocm-sdk
Remove old WIF models and custom client code and replace them with the sdk
Update all reference to the old models in cmd/ocm/gcp with the official SDK models and client

It is not intended to introduce changes in functionality; however some changes were necessary to handle custom roles.

JakobGray commented 2 months ago

@ckandag @renan-campos

JakobGray commented 2 months ago

I found issues with the wif templates that need to be addressed before this is merged

ckandag commented 2 months ago

@JakobGray : Across the board please fix error handling to be similar to be rest of ocm cli. Please remove all the Fatal stmt and bubble errors up to caller. This allows for the error handling/ display to be addressed better and consistently across the CLI

JakobGray commented 2 months ago

@JakobGray : Across the board please fix error handling to be similar to be rest of ocm cli. Please remove all the Fatal stmt and bubble errors up to caller. This allows for the error handling/ display to be addressed better and consistently across the CLI

Because this PR is not intended to introduce changes in functionality I don't want to extend it's scope to address all error handling. I believe we have a ticket already to address error handling more gracefully so I think that kind of change is better addressed there.

ckandag commented 2 months ago

@JakobGray : Across the board please fix error handling to be similar to be rest of ocm cli. Please remove all the Fatal stmt and bubble errors up to caller. This allows for the error handling/ display to be addressed better and consistently across the CLI

Because this PR is not intended to introduce changes in functionality I don't want to extend it's scope to address all error handling. I believe we have a ticket already to address error handling more gracefully so I think that kind of change is better addressed there.

@JakobGray I raised this in the original MR as well https://github.com/openshift-online/ocm-cli/pull/619#discussion_r1656407297

Can we please sort this out now rather than later you are anyway touching all the same files , may be you can do it in a separate commit . I am not sure why we decided to let this go during the initial checkin, now seems like the time to get this right.

JakobGray commented 2 months ago

The original MR addressed it by removing any panics and surfacing all errors to the top level function. This latest commit takes it a step further by removing all log.Fatal's from the top level function as well.

renan-campos commented 2 months ago

ocm-gcp-create-wif-config.log