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.

Sign up for GitHub

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

Jump to bottom

[WIP] Remove deprecated KubeException #293

Closed
wants to merge 1 commit into from
Closed

[WIP] Remove deprecated KubeException #293

wants to merge 1 commit into from

Conversation

moolitayer
Copy link
Collaborator

@moolitayer moolitayer commented Jan 23, 2018 •
edited
Loading

See discussion in #195

Part of: #199

@moolitayer moolitayer force-pushed the remove_kubeexception branch 3 times, most recently from 2f756a0 to 3ca4d41 Compare January 24, 2018 15:01
@moolitayer moolitayer changed the title [WIP] Remove deprecated KubeException Remove deprecated KubeException Jan 24, 2018
@moolitayer
Copy link
Collaborator Author

@cben @grosser @kirs please review

@moolitayer
Copy link
Collaborator Author

@jhernand please take a look

jhernand
jhernand suggested changes Jan 25, 2018
View reviewed changes
README.md Outdated
### past version 3.0

Deprecated KubeException has been dropped, use `Kubeclient::HttpException` instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be Kubeclient::HttpError.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

see #288

@jhernand
Copy link
Contributor

@pkliczewski, @masayag the KubeVirt provider uses the deprecated exception:

https://github.com/ManageIQ/manageiq-providers-kubevirt/blob/d46aca33eb6330b4e79a16100c6379e53a912ab5/app/models/manageiq/providers/kubevirt/infra_manager/refresh_worker/runner.rb#L192

Makes sure to change that to Kubeclient::HttpError before upgrading to the upcoming version 3.0 of kubeclient. You can change it now, if you want, as version 2.4 already supports it.

@moolitayer moolitayer force-pushed the remove_kubeexception branch from 3ca4d41 to aff5721 Compare January 25, 2018 08:43
@moolitayer moolitayer force-pushed the remove_kubeexception branch from aff5721 to 886d292 Compare January 25, 2018 08:47
@moolitayer
Copy link
Collaborator Author

@cben please review

This was referenced Jan 25, 2018
Closed
Merged
@moolitayer moolitayer changed the title Remove deprecated KubeException [WIP] Remove deprecated KubeException Jan 31, 2018
@moolitayer
Copy link
Collaborator Author

Discussed with @cben
Planning to release maser, as we haven't yet released a version where this is deprecated we should wait with this one.

@timothysmith0609 timothysmith0609 mentioned this pull request Jan 25, 2019
Merged
@cben
Copy link
Collaborator

cben commented Feb 26, 2019

status: I've been postponing this as I see an increasing needs for errors where HttpError would make no sense, specifically around Config. Plus if we'll get auth auto-renewal (#393), we might start seeing Config-related errors from Client methods (get_pods etc) that currently only raise HttpError.

All this leads to me to think kubeclient still needs a top class/module covering all its errors.
For now that's KubeException but we might want to transition to say Kubeclient::Error.
As Config errors are currently various RuntimeError, KeyError etc, I want to figure out if it's reasonable to substitute a different class...
=> So, still stalling on this... 🚧

@cben cben mentioned this pull request Mar 10, 2020
Open
11 tasks
@moolitayer moolitayer closed this Jul 21, 2020
@cben cben mentioned this pull request Nov 16, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grosser grosser grosser approved these changes

@kirs kirs kirs approved these changes

@jhernand jhernand jhernand approved these changes

Assignees

@cben cben

Labels
v2.x/no
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@moolitayer @jhernand @cben @grosser @kirs