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

added ErrorStatus #875

Merged
merged 3 commits into from
Jan 5, 2023
Merged

added ErrorStatus #875

merged 3 commits into from
Jan 5, 2023

Conversation

PietroPasotti
Copy link
Contributor

Added ErrorStatus so that getting Unit/App.status when the status is "error" does not raise.

Checklist

  • Have any types changed? If so, have the type annotations been updated?
  • If this code exposes model data, does it do so thoughtfully, in a way that aids understanding?
  • Do error messages, if any, present charm authors or operators with enough information to solve the problem?

QA steps

added tests

Documentation changes

We should make it clear that there's two statuses that can't be set now: Unknown and Error.

Bug reference

Fixes #874

Changelog

@benhoyt benhoyt self-requested a review January 4, 2023 22:49
benhoyt
benhoyt approved these changes Jan 4, 2023
View reviewed changes
Copy link
Collaborator

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Thanks for this, @PietroPasotti. I've made a few simplifications/tweaks and pushed to your branch.

@benhoyt benhoyt requested a review from jameinel January 4, 2023 23:19
@benhoyt benhoyt mentioned this pull request Jan 4, 2023
Closed
arturo-seijas
arturo-seijas approved these changes Jan 5, 2023
View reviewed changes
Copy link

@arturo-seijas arturo-seijas left a comment

Choose a reason for hiding this comment

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

LGTM

@benhoyt benhoyt merged commit 31e2220 into canonical:main Jan 5, 2023
mkalcok pushed a commit to mkalcok/operator that referenced this pull request Apr 24, 2023
@PietroPasotti @benhoyt @mkalcok
Add ErrorStatus (canonical#875)
74ccecd 
Add ErrorStatus so that getting Unit/App.status when the status is
"error" does not raise.

Co-authored-by: Ben Hoyt <ben.hoyt@canonical.com>
@mkalcok mkalcok mentioned this pull request Apr 24, 2023
Merged
3 tasks
mkalcok pushed a commit to mkalcok/operator that referenced this pull request Apr 25, 2023
@PietroPasotti @benhoyt @mkalcok
Add ErrorStatus (canonical#875)
fe7e40c 
Add ErrorStatus so that getting Unit/App.status when the status is
"error" does not raise.

Co-authored-by: Ben Hoyt <ben.hoyt@canonical.com>
benhoyt added a commit that referenced this pull request Apr 25, 2023
@mkalcok @PietroPasotti @benhoyt
Backport ErrorStatus to 1.5 branch (#875) (#927)
5bbc50f 
Add ErrorStatus so that getting Unit/App.status when the status is
"error" does not raise.

Co-authored-by: PietroPasotti <starfire.daemon@gmail.com>
Co-authored-by: Ben Hoyt <ben.hoyt@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jnsgruk jnsgruk jnsgruk approved these changes

@benhoyt benhoyt benhoyt approved these changes

@arturo-seijas arturo-seijas arturo-seijas approved these changes

@jameinel jameinel Awaiting requested review from jameinel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error status causes update-status hook to fail
4 participants
@PietroPasotti @jnsgruk @benhoyt @arturo-seijas