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

Operational limit setters #3060

Open
colinepiloquet opened this issue Jun 7, 2024 · 4 comments · May be fixed by #3113
Open

Operational limit setters #3060

colinepiloquet opened this issue Jun 7, 2024 · 4 comments · May be fixed by #3113
Assignees

Comments

@colinepiloquet
Copy link
Contributor

Describe the current behavior

There are no setters for TemporaryLimits which means that to modify one value the whole limit must be recreated.

Describe the expected behavior

I think it would we useful to be able to set some of the attributes of the temporary limits, especially the value.

Describe the motivation

It would be useful to correct faulty data without having to store all the temporary limits and recreate the whole limit.

Extra Information

No response

@colinepiloquet
Copy link
Contributor Author

@annetill, @HugoKulesza, a good first issue.

@nao1345678 nao1345678 self-assigned this Jun 10, 2024
@nao1345678
Copy link
Contributor

To correct an already existing limit, we have to make sure the new value fits logically in between the already existing other temporary limits.
For instance, if the new value is greather than the next one, shouldn't we change the acceptable duration too ? Or should we change the limits' order ?

@colinepiloquet
Copy link
Contributor Author

Maybe it's enough to check the other values and if the new limit does not fit logically then the correction would not work ?

@nao1345678 nao1345678 linked a pull request Aug 1, 2024 that will close this issue
5 tasks
@olperr1
Copy link
Member

olperr1 commented Aug 6, 2024

When a LoadingLimits is created, we only log warnings when the temporary limits' values are not consistent (see ValidationUtil.checkTemporaryLimits(...)).
I think that we can adopt the same principle: allow the inconsistent values, but log warnings in that case.

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.

3 participants