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

Remove arbitrary priority validation #19

Open
samuelallan72 opened this issue Aug 15, 2022 · 0 comments
Open

Remove arbitrary priority validation #19

samuelallan72 opened this issue Aug 15, 2022 · 0 comments

Comments

@samuelallan72
Copy link

            if priority <= 0:
                raise TypeError(f"priority needs to be > 0, not {priority}")

and

        if status.priority and status.priority > 100:
            raise ValueError("Status priority cannot be > 100")

These are arbitrary constraints - nothing cares about the value, as long as they're valid floats or ints that can be sorted. Introduces confusion, and potential unexpected crashes:

  • Instantiating a Status with priority=200 will succeed, but adding to the pool later will raise an error
  • Instantiating 102+ statuses with auto priority will work, but then adding any new status to the pool will crash
  • Adding 102+ statuses with manual priorities within these bounds will succeed, but restoring from the charm storage will crash (see Save status priority to charm state #15 )

These may be unlikely, but not impossible.

Remove this validation.

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 a pull request may close this issue.

1 participant